fsnotify: do not share events between notification groups

Currently fsnotify framework creates one event structure for each
notification event and links this event into all interested notification
groups.  This is done so that we save memory when several notification
groups are interested in the event.  However the need for event
structure shared between inotify & fanotify bloats the event structure
so the result is often higher memory consumption.

Another problem is that fsnotify framework keeps path references with
outstanding events so that fanotify can return open file descriptors
with its events.  This has the undesirable effect that filesystem cannot
be unmounted while there are outstanding events - a regression for
inotify compared to a situation before it was converted to fsnotify
framework.  For fanotify this problem is hard to avoid and users of
fanotify should kind of expect this behavior when they ask for file
descriptors from notified files.

This patch changes fsnotify and its users to create separate event
structure for each group.  This allows for much simpler code (~400 lines
removed by this patch) and also smaller event structures.  For example
on 64-bit system original struct fsnotify_event consumes 120 bytes, plus
additional space for file name, additional 24 bytes for second and each
subsequent group linking the event, and additional 32 bytes for each
inotify group for private data.  After the conversion inotify event
consumes 48 bytes plus space for file name which is considerably less
memory unless file names are long and there are several groups
interested in the events (both of which are uncommon).  Fanotify event
fits in 56 bytes after the conversion (fanotify doesn't care about file
names so its events don't have to have it allocated).  A win unless
there are four or more fanotify groups interested in the event.

The conversion also solves the problem with unmount when only inotify is
used as we don't have to grab path references for inotify events.

[hughd@google.com: fanotify: fix corruption preventing startup]
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 12 insertions, 25 deletions

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 4bb21d67d9b1..7c754c91c3f6 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -128,8 +128,7 @@ static int send_to_group(struct inode *to_tell,
                          struct fsnotify_mark *vfsmount_mark,
                          __u32 mask, void *data,
                          int data_is, u32 cookie,
-                         const unsigned char *file_name,
-                         struct fsnotify_event **event)
+                         const unsigned char *file_name)
 {
         struct fsnotify_group *group = NULL;
         __u32 inode_test_mask = 0;
@@ -170,10 +169,10 @@ static int send_to_group(struct inode *to_tell,
 
         pr_debug("%s: group=%p to_tell=%p mask=%x inode_mark=%p"
                  " inode_test_mask=%x vfsmount_mark=%p vfsmount_test_mask=%x"
-                 " data=%p data_is=%d cookie=%d event=%p\n",
+                 " data=%p data_is=%d cookie=%d\n",
                  __func__, group, to_tell, mask, inode_mark,
                  inode_test_mask, vfsmount_mark, vfsmount_test_mask, data,
-                 data_is, cookie, *event);
+                 data_is, cookie);
 
         if (!inode_test_mask && !vfsmount_test_mask)
                 return 0;
@@ -183,14 +182,9 @@ static int send_to_group(struct inode *to_tell,
                                           data_is) == false)
                 return 0;
 
-        if (!*event) {
-                *event = fsnotify_create_event(to_tell, mask, data,
-                                                data_is, file_name,
-                                                cookie, GFP_KERNEL);
-                if (!*event)
-                        return -ENOMEM;
-        }
-        return group->ops->handle_event(group, inode_mark, vfsmount_mark, *event);
+        return group->ops->handle_event(group, to_tell, inode_mark,
+                                        vfsmount_mark, mask, data, data_is,
+                                        file_name);
 }
 
 /*
@@ -205,7 +199,6 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is,
         struct hlist_node *inode_node = NULL, *vfsmount_node = NULL;
         struct fsnotify_mark *inode_mark = NULL, *vfsmount_mark = NULL;
         struct fsnotify_group *inode_group, *vfsmount_group;
-        struct fsnotify_event *event = NULL;
         struct mount *mnt;
         int idx, ret = 0;
         /* global tests shouldn't care about events on child only the specific event */
@@ -258,18 +251,18 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is,
 
                 if (inode_group > vfsmount_group) {
                         /* handle inode */
-                        ret = send_to_group(to_tell, inode_mark, NULL, mask, data,
-                                            data_is, cookie, file_name, &event);
+                        ret = send_to_group(to_tell, inode_mark, NULL, mask,
+                                            data, data_is, cookie, file_name);
                         /* we didn't use the vfsmount_mark */
                         vfsmount_group = NULL;
                 } else if (vfsmount_group > inode_group) {
-                        ret = send_to_group(to_tell, NULL, vfsmount_mark, mask, data,
-                                            data_is, cookie, file_name, &event);
+                        ret = send_to_group(to_tell, NULL, vfsmount_mark, mask,
+                                            data, data_is, cookie, file_name);
                         inode_group = NULL;
                 } else {
                         ret = send_to_group(to_tell, inode_mark, vfsmount_mark,
-                                            mask, data, data_is, cookie, file_name,
-                                            &event);
+                                            mask, data, data_is, cookie,
+                                            file_name);
                 }
 
                 if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
@@ -285,12 +278,6 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is,
         ret = 0;
 out:
         srcu_read_unlock(&fsnotify_mark_srcu, idx);
-        /*
-         * fsnotify_create_event() took a reference so the event can't be cleaned
-         * up while we are still trying to add it to lists, drop that one.
-         */
-        if (event)
-                fsnotify_put_event(event);
 
         return ret;
 }