From: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>

This reminded me... you have two pr_debugs in fanotify_should_send_event
which output redundant information. Maybe you intended it like that so
it is selectable how much log spam you want, or if not you may want to
apply this patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
 fs/notify/fanotify/fanotify.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 756566f..85366c7 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -165,9 +165,6 @@ static bool fanotify_should_send_event(struct fsnotify_group *group,
 		 "mask=%x data=%p data_type=%d\n", __func__, group, to_tell,
 		 inode_mark, vfsmnt_mark, event_mask, data, data_type);
 
-	pr_debug("%s: group=%p vfsmount_mark=%p inode_mark=%p mask=%x\n",
-		 __func__, group, vfsmnt_mark, inode_mark, event_mask);
-
 	/* sorry, fanotify only gives a damn about files and dirs */
 	if (!S_ISREG(to_tell->i_mode) &&
 	    !S_ISDIR(to_tell->i_mode))
-- 
1.6.5.3

--

[ message continues ]

From: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>

resize pid and reorder the fanotify_event_metadata so it is naturally
aligned and we can work towards dropping the packed attributed

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
Cc: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
 include/linux/fanotify.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 9854356..0535461 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -70,9 +70,9 @@
 struct fanotify_event_metadata {
 	__u32 event_len;
 	__u32 vers;
-	__s32 fd;
 	__u64 mask;
-	__s64 pid;
+	__s32 fd;
+	__s32 pid;
 } __attribute__ ((packed));
 
 struct fanotify_response {
-- 
1.6.5.3

--

[ message continues ]

The userspace event metadata structure was packed so when sent from a kernel
with a certain set of alignment rules to a userspace listener with a different
set of alignment rules the userspace process would be able to use the
structure.  On some arches just using packed, even if it doesn't do anything
to the alignment can cause a severe performance hit.  From now on we are
not going to set the packed attribute and will just need to be very careful
to make sure the structure is naturally aligned and that explicit padding is
used when necessary.  To make sure noone gets this wrong in the future, we
enforce this fact, at build time, using a similar structure that is packed
and comparing their sizes.

Cc: Andreas Dilger <adilger@dilger.ca>
Cc: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
 fs/notify/fanotify/fanotify_user.c |    2 ++
 include/linux/fanotify.h           |   22 ++++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index b966b72..c3a5742 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -778,6 +778,8 @@ SYSCALL_ALIAS(sys_fanotify_mark, SyS_fanotify_mark);
  */
 static int __init fanotify_user_setup(void)
 {
+	BUILD_BUG_ON(sizeof(struct fanotify_event_metadata) !=
+		     sizeof(struct fan_event_meta_packed));
 	fanotify_mark_cache = KMEM_CACHE(fsnotify_mark, SLAB_PANIC);
 	fanotify_response_event_cache = KMEM_CACHE(fanotify_response_event,
 						   SLAB_PANIC);
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 0535461..3d7a9b5 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -66,14 +66,21 @@
 				 FAN_Q_OVERFLOW)
 
 #define FANOTIFY_METADATA_VERSION	1
-
+/*
+ * This structue must be naturally aligned so that a 32 bit userspace process
+ * will find the offsets the same as a 64bit process.  If there would be padding
+ * ...
[ message continues ]

We don't actually care if there are any holes in this structure; all we
care about is that the structure has the same alignment on 32-bit and
64-bit architectures.


This does not add much value; the two structures can still go out of sync
A note to be careful about changes to struct fanotify_event_metadata
should really be warning enough.

Andreas
--

[ message continues ]