From 7053aee26a3548ebaba046ae2e52396ccf56ac6c Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 21 Jan 2014 15:48:14 -0800 Subject: 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 Reviewed-by: Christoph Hellwig Cc: Eric Paris Cc: Al Viro Signed-off-by: Hugh Dickins Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/audit_tree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'kernel/audit_tree.c') diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 43c307dc945..bcc0b182122 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -912,9 +912,11 @@ static void evict_chunk(struct audit_chunk *chunk) } static int audit_tree_handle_event(struct fsnotify_group *group, + struct inode *to_tell, struct fsnotify_mark *inode_mark, - struct fsnotify_mark *vfsmonut_mark, - struct fsnotify_event *event) + struct fsnotify_mark *vfsmount_mark, + u32 mask, void *data, int data_type, + const unsigned char *file_name) { BUG(); return -EOPNOTSUPP; @@ -945,7 +947,7 @@ static const struct fsnotify_ops audit_tree_ops = { .handle_event = audit_tree_handle_event, .should_send_event = audit_tree_send_event, .free_group_priv = NULL, - .free_event_priv = NULL, + .free_event = NULL, .freeing_mark = audit_tree_freeing_mark, }; -- cgit v1.2.3-70-g09d2 From 83c4c4b0a3aadc1ce7b5b2870ce1fc1f65498da0 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 21 Jan 2014 15:48:15 -0800 Subject: fsnotify: remove .should_send_event callback After removing event structure creation from the generic layer there is no reason for separate .should_send_event and .handle_event callbacks. So just remove the first one. Signed-off-by: Jan Kara Reviewed-by: Christoph Hellwig Cc: Eric Paris Cc: Al Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/dnotify/dnotify.c | 22 ++++------------------ fs/notify/fanotify/fanotify.c | 18 ++++++++++-------- fs/notify/fsnotify.c | 5 ----- fs/notify/inotify/inotify_fsnotify.c | 24 +++++++----------------- include/linux/fsnotify_backend.h | 4 ---- kernel/audit_tree.c | 12 +----------- kernel/audit_watch.c | 9 --------- 7 files changed, 22 insertions(+), 72 deletions(-) (limited to 'kernel/audit_tree.c') diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index bfca53dbbf3..928688e3ee2 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -94,6 +94,10 @@ static int dnotify_handle_event(struct fsnotify_group *group, struct fown_struct *fown; __u32 test_mask = mask & ~FS_EVENT_ON_CHILD; + /* not a dir, dnotify doesn't care */ + if (!S_ISDIR(inode->i_mode)) + return 0; + BUG_ON(vfsmount_mark); dn_mark = container_of(inode_mark, struct dnotify_mark, fsn_mark); @@ -121,23 +125,6 @@ static int dnotify_handle_event(struct fsnotify_group *group, return 0; } -/* - * Given an inode and mask determine if dnotify would be interested in sending - * userspace notification for that pair. - */ -static bool dnotify_should_send_event(struct fsnotify_group *group, - struct inode *inode, - struct fsnotify_mark *inode_mark, - struct fsnotify_mark *vfsmount_mark, - __u32 mask, void *data, int data_type) -{ - /* not a dir, dnotify doesn't care */ - if (!S_ISDIR(inode->i_mode)) - return false; - - return true; -} - static void dnotify_free_mark(struct fsnotify_mark *fsn_mark) { struct dnotify_mark *dn_mark = container_of(fsn_mark, @@ -151,7 +138,6 @@ static void dnotify_free_mark(struct fsnotify_mark *fsn_mark) static struct fsnotify_ops dnotify_fsnotify_ops = { .handle_event = dnotify_handle_event, - .should_send_event = dnotify_should_send_event, .free_group_priv = NULL, .freeing_mark = NULL, .free_event = NULL, diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index c26268d7bd9..1f8f05220f8 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -88,18 +88,17 @@ static int fanotify_get_response_from_access(struct fsnotify_group *group, } #endif -static bool fanotify_should_send_event(struct fsnotify_group *group, - struct inode *inode, - struct fsnotify_mark *inode_mark, +static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark, struct fsnotify_mark *vfsmnt_mark, - __u32 event_mask, void *data, int data_type) + u32 event_mask, + void *data, int data_type) { __u32 marks_mask, marks_ignored_mask; struct path *path = data; - pr_debug("%s: group=%p inode=%p inode_mark=%p vfsmnt_mark=%p " - "mask=%x data=%p data_type=%d\n", __func__, group, inode, - inode_mark, vfsmnt_mark, event_mask, data, data_type); + pr_debug("%s: inode_mark=%p vfsmnt_mark=%p mask=%x data=%p" + " data_type=%d\n", __func__, inode_mark, vfsmnt_mark, + event_mask, data, data_type); /* if we don't have enough info to send an event to userspace say no */ if (data_type != FSNOTIFY_EVENT_PATH) @@ -163,6 +162,10 @@ static int fanotify_handle_event(struct fsnotify_group *group, BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM); BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR); + if (!fanotify_should_send_event(inode_mark, fanotify_mark, mask, data, + data_type)) + return 0; + pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode, mask); @@ -225,7 +228,6 @@ static void fanotify_free_event(struct fsnotify_event *fsn_event) const struct fsnotify_ops fanotify_fsnotify_ops = { .handle_event = fanotify_handle_event, - .should_send_event = fanotify_should_send_event, .free_group_priv = fanotify_free_group_priv, .free_event = fanotify_free_event, .freeing_mark = NULL, diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 7c754c91c3f..1d4e1ea2f37 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -177,11 +177,6 @@ static int send_to_group(struct inode *to_tell, if (!inode_test_mask && !vfsmount_test_mask) return 0; - if (group->ops->should_send_event(group, to_tell, inode_mark, - vfsmount_mark, mask, data, - data_is) == false) - return 0; - return group->ops->handle_event(group, to_tell, inode_mark, vfsmount_mark, mask, data, data_is, file_name); diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c index 6fabbd163d1..aad1a35e9af 100644 --- a/fs/notify/inotify/inotify_fsnotify.c +++ b/fs/notify/inotify/inotify_fsnotify.c @@ -81,6 +81,13 @@ int inotify_handle_event(struct fsnotify_group *group, BUG_ON(vfsmount_mark); + if ((inode_mark->mask & FS_EXCL_UNLINK) && + (data_type == FSNOTIFY_EVENT_PATH)) { + struct path *path = data; + + if (d_unlinked(path->dentry)) + return 0; + } if (file_name) { len = strlen(file_name); alloc_len += len + 1; @@ -122,22 +129,6 @@ static void inotify_freeing_mark(struct fsnotify_mark *fsn_mark, struct fsnotify inotify_ignored_and_remove_idr(fsn_mark, group); } -static bool inotify_should_send_event(struct fsnotify_group *group, struct inode *inode, - struct fsnotify_mark *inode_mark, - struct fsnotify_mark *vfsmount_mark, - __u32 mask, void *data, int data_type) -{ - if ((inode_mark->mask & FS_EXCL_UNLINK) && - (data_type == FSNOTIFY_EVENT_PATH)) { - struct path *path = data; - - if (d_unlinked(path->dentry)) - return false; - } - - return true; -} - /* * This is NEVER supposed to be called. Inotify marks should either have been * removed from the idr when the watch was removed or in the @@ -189,7 +180,6 @@ static void inotify_free_event(struct fsnotify_event *fsn_event) const struct fsnotify_ops inotify_fsnotify_ops = { .handle_event = inotify_handle_event, - .should_send_event = inotify_should_send_event, .free_group_priv = inotify_free_group_priv, .free_event = inotify_free_event, .freeing_mark = inotify_freeing_mark, diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 7f3d7dcfcd0..7d8d5e60859 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -94,10 +94,6 @@ struct fsnotify_fname; * userspace messages that marks have been removed. */ struct fsnotify_ops { - bool (*should_send_event)(struct fsnotify_group *group, struct inode *inode, - struct fsnotify_mark *inode_mark, - 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, diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index bcc0b182122..ae8103b057f 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -918,8 +918,7 @@ static int audit_tree_handle_event(struct fsnotify_group *group, u32 mask, void *data, int data_type, const unsigned char *file_name) { - BUG(); - return -EOPNOTSUPP; + return 0; } static void audit_tree_freeing_mark(struct fsnotify_mark *entry, struct fsnotify_group *group) @@ -935,17 +934,8 @@ static void audit_tree_freeing_mark(struct fsnotify_mark *entry, struct fsnotify BUG_ON(atomic_read(&entry->refcnt) < 1); } -static bool audit_tree_send_event(struct fsnotify_group *group, struct inode *inode, - struct fsnotify_mark *inode_mark, - struct fsnotify_mark *vfsmount_mark, - __u32 mask, void *data, int data_type) -{ - return false; -} - static const struct fsnotify_ops audit_tree_ops = { .handle_event = audit_tree_handle_event, - .should_send_event = audit_tree_send_event, .free_group_priv = NULL, .free_event = NULL, .freeing_mark = audit_tree_freeing_mark, diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index a760c32cb63..367ac9a79ac 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -465,14 +465,6 @@ void audit_remove_watch_rule(struct audit_krule *krule) } } -static bool audit_watch_should_send_event(struct fsnotify_group *group, struct inode *inode, - struct fsnotify_mark *inode_mark, - struct fsnotify_mark *vfsmount_mark, - __u32 mask, void *data, int data_type) -{ - return true; -} - /* Update watch data in audit rules based on fsnotify events. */ static int audit_watch_handle_event(struct fsnotify_group *group, struct inode *to_tell, @@ -512,7 +504,6 @@ static int audit_watch_handle_event(struct fsnotify_group *group, } static const struct fsnotify_ops audit_watch_fsnotify_ops = { - .should_send_event = audit_watch_should_send_event, .handle_event = audit_watch_handle_event, .free_group_priv = NULL, .freeing_mark = NULL, -- cgit v1.2.3-70-g09d2 From 56b27cf6030dd36c56a5542ab8bfa406d337f083 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 21 Jan 2014 15:48:16 -0800 Subject: fsnotify: remove pointless NULL initializers We usually rely on the fact that struct members not specified in the initializer are set to NULL. So do that with fsnotify function pointers as well. Signed-off-by: Jan Kara Reviewed-by: Christoph Hellwig Cc: Eric Paris Cc: Al Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/dnotify/dnotify.c | 3 --- fs/notify/fanotify/fanotify.c | 1 - kernel/audit_tree.c | 2 -- kernel/audit_watch.c | 3 --- 4 files changed, 9 deletions(-) (limited to 'kernel/audit_tree.c') diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 928688e3ee2..0b9ff4395e6 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -138,9 +138,6 @@ static void dnotify_free_mark(struct fsnotify_mark *fsn_mark) static struct fsnotify_ops dnotify_fsnotify_ops = { .handle_event = dnotify_handle_event, - .free_group_priv = NULL, - .freeing_mark = NULL, - .free_event = NULL, }; /* diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 1f8f05220f8..58772623f02 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -230,5 +230,4 @@ const struct fsnotify_ops fanotify_fsnotify_ops = { .handle_event = fanotify_handle_event, .free_group_priv = fanotify_free_group_priv, .free_event = fanotify_free_event, - .freeing_mark = NULL, }; diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index ae8103b057f..67ccf0e7cca 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -936,8 +936,6 @@ static void audit_tree_freeing_mark(struct fsnotify_mark *entry, struct fsnotify static const struct fsnotify_ops audit_tree_ops = { .handle_event = audit_tree_handle_event, - .free_group_priv = NULL, - .free_event = NULL, .freeing_mark = audit_tree_freeing_mark, }; diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 367ac9a79ac..2596fac5dcb 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -505,9 +505,6 @@ static int audit_watch_handle_event(struct fsnotify_group *group, static const struct fsnotify_ops audit_watch_fsnotify_ops = { .handle_event = audit_watch_handle_event, - .free_group_priv = NULL, - .freeing_mark = NULL, - .free_event = NULL, }; static int __init audit_watch_init(void) -- cgit v1.2.3-70-g09d2