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, 27 insertions, 87 deletions

diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 4b2ee8d12f5e..7f3d7dcfcd00 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -15,7 +15,6 @@
 #include <linux/path.h> /* struct path */
 #include <linux/spinlock.h>
 #include <linux/types.h>
-
 #include <linux/atomic.h>
 
 /*
@@ -79,6 +78,7 @@ struct fsnotify_group;
 struct fsnotify_event;
 struct fsnotify_mark;
 struct fsnotify_event_private_data;
+struct fsnotify_fname;
 
 /*
  * Each group much define these ops.  The fsnotify infrastructure will call
@@ -99,12 +99,26 @@ struct fsnotify_ops {
                                   struct fsnotify_mark *vfsmount_mark,
                                   __u32 mask, void *data, int data_type);
         int (*handle_event)(struct fsnotify_group *group,
+                            struct inode *inode,
                             struct fsnotify_mark *inode_mark,
                             struct fsnotify_mark *vfsmount_mark,
-                            struct fsnotify_event *event);
+                            u32 mask, void *data, int data_type,
+                            const unsigned char *file_name);
         void (*free_group_priv)(struct fsnotify_group *group);
         void (*freeing_mark)(struct fsnotify_mark *mark, struct fsnotify_group *group);
-        void (*free_event_priv)(struct fsnotify_event_private_data *priv);
+        void (*free_event)(struct fsnotify_event *event);
+};
+
+/*
+ * all of the information about the original object we want to now send to
+ * a group.  If you want to carry more info from the accessing task to the
+ * listener this structure is where you need to be adding fields.
+ */
+struct fsnotify_event {
+        struct list_head list;
+        /* inode may ONLY be dereferenced during handle_event(). */
+        struct inode *inode;    /* either the inode the event happened to or its parent */
+        u32 mask;               /* the type of access, bitwise OR for FS_* event types */
 };
 
 /*
@@ -148,7 +162,11 @@ struct fsnotify_group {
                                          * a group */
         struct list_head marks_list;    /* all inode marks for this group */
 
-        struct fasync_struct    *fsn_fa;    /* async notification */
+        struct fasync_struct *fsn_fa;    /* async notification */
+
+        struct fsnotify_event overflow_event;   /* Event we queue when the
+                                                 * notification list is too
+                                                 * full */
 
         /* groups can define private fields here or use the void *private */
         union {
@@ -177,76 +195,10 @@ struct fsnotify_group {
         };
 };
 
-/*
- * A single event can be queued in multiple group->notification_lists.
- *
- * each group->notification_list will point to an event_holder which in turns points
- * to the actual event that needs to be sent to userspace.
- *
- * Seemed cheaper to create a refcnt'd event and a small holder for every group
- * than create a different event for every group
- *
- */
-struct fsnotify_event_holder {
-        struct fsnotify_event *event;
-        struct list_head event_list;
-};
-
-/*
- * Inotify needs to tack data onto an event.  This struct lets us later find the
- * correct private data of the correct group.
- */
-struct fsnotify_event_private_data {
-        struct fsnotify_group *group;
-        struct list_head event_list;
-};
-
-/*
- * all of the information about the original object we want to now send to
- * a group.  If you want to carry more info from the accessing task to the
- * listener this structure is where you need to be adding fields.
- */
-struct fsnotify_event {
-        /*
-         * If we create an event we are also likely going to need a holder
-         * to link to a group.  So embed one holder in the event.  Means only
-         * one allocation for the common case where we only have one group
-         */
-        struct fsnotify_event_holder holder;
-        spinlock_t lock;        /* protection for the associated event_holder and private_list */
-        /* to_tell may ONLY be dereferenced during handle_event(). */
-        struct inode *to_tell;  /* either the inode the event happened to or its parent */
-        /*
-         * depending on the event type we should have either a path or inode
-         * We hold a reference on path, but NOT on inode.  Since we have the ref on
-         * the path, it may be dereferenced at any point during this object's
-         * lifetime.  That reference is dropped when this object's refcnt hits
-         * 0.  If this event contains an inode instead of a path, the inode may
-         * ONLY be used during handle_event().
-         */
-        union {
-                struct path path;
-                struct inode *inode;
-        };
 /* when calling fsnotify tell it if the data is a path or inode */
 #define FSNOTIFY_EVENT_NONE     0
 #define FSNOTIFY_EVENT_PATH     1
 #define FSNOTIFY_EVENT_INODE    2
-        int data_type;          /* which of the above union we have */
-        atomic_t refcnt;        /* how many groups still are using/need to send this event */
-        __u32 mask;             /* the type of access, bitwise OR for FS_* event types */
-
-        u32 sync_cookie;        /* used to corrolate events, namely inotify mv events */
-        const unsigned char *file_name;
-        size_t name_len;
-        struct pid *tgid;
-
-#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
-        __u32 response; /* userspace answer to question */
-#endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */
-
-        struct list_head private_data_list;     /* groups can store private data here */
-};
 
 /*
  * Inode specific fields in an fsnotify_mark
@@ -370,17 +322,12 @@ extern void fsnotify_put_group(struct fsnotify_group *group);
 extern void fsnotify_destroy_group(struct fsnotify_group *group);
 /* fasync handler function */
 extern int fsnotify_fasync(int fd, struct file *file, int on);
-/* take a reference to an event */
-extern void fsnotify_get_event(struct fsnotify_event *event);
-extern void fsnotify_put_event(struct fsnotify_event *event);
-/* find private data previously attached to an event and unlink it */
-extern struct fsnotify_event_private_data *fsnotify_remove_priv_from_event(struct fsnotify_group *group,
-                                                                           struct fsnotify_event *event);
-
+/* Free event from memory */
+extern void fsnotify_destroy_event(struct fsnotify_group *group,
+                                   struct fsnotify_event *event);
 /* attach the event to the group notification queue */
 extern struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
                                                         struct fsnotify_event *event,
-                                                        struct fsnotify_event_private_data *priv,
                                                         struct fsnotify_event *(*merge)(struct list_head *,
                                                                                         struct fsnotify_event *));
 /* true if the group notification queue is empty */
@@ -430,15 +377,8 @@ extern void fsnotify_put_mark(struct fsnotify_mark *mark);
 extern void fsnotify_unmount_inodes(struct list_head *list);
 
 /* put here because inotify does some weird stuff when destroying watches */
-extern struct fsnotify_event *fsnotify_create_event(struct inode *to_tell, __u32 mask,
-                                                    void *data, int data_is,
-                                                    const unsigned char *name,
-                                                    u32 cookie, gfp_t gfp);
-
-/* fanotify likes to change events after they are on lists... */
-extern struct fsnotify_event *fsnotify_clone_event(struct fsnotify_event *old_event);
-extern int fsnotify_replace_event(struct fsnotify_event_holder *old_holder,
-                                  struct fsnotify_event *new_event);
+extern void fsnotify_init_event(struct fsnotify_event *event,
+                                struct inode *to_tell, u32 mask);
 
 #else