Commit 7053aee2 authored by Jan Kara's avatar Jan Kara Committed by Linus Torvalds
Browse files

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: default avatarJan Kara <jack@suse.cz>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarHugh Dickins <hughd@google.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent e9fe6904

fs/notify/dnotify/dnotify.c

+5 −6
Original line number Diff line number Diff line
@@ -82,21 +82,20 @@ static void dnotify_recalc_inode_mask(struct fsnotify_mark *fsn_mark) 
 * events.

 */

static int dnotify_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)

{

	struct dnotify_mark *dn_mark;

	struct inode *to_tell;

	struct dnotify_struct *dn;

	struct dnotify_struct **prev;

	struct fown_struct *fown;

	__u32 test_mask = event->mask & ~FS_EVENT_ON_CHILD;

	__u32 test_mask = mask & ~FS_EVENT_ON_CHILD;



	BUG_ON(vfsmount_mark);



	to_tell = event->to_tell;



	dn_mark = container_of(inode_mark, struct dnotify_mark, fsn_mark);



	spin_lock(&inode_mark->lock);
@@ -155,7 +154,7 @@ static struct fsnotify_ops dnotify_fsnotify_ops = { 
	.should_send_event = dnotify_should_send_event,

	.free_group_priv = NULL,

	.freeing_mark = NULL,

	.free_event_priv = NULL,

	.free_event = NULL,

};



/*

fs/notify/fanotify/fanotify.c

+104 −107
Original line number Diff line number Diff line
@@ -9,31 +9,27 @@ 
#include <linux/types.h>

#include <linux/wait.h>



static bool should_merge(struct fsnotify_event *old, struct fsnotify_event *new)

#include "fanotify.h"



static bool should_merge(struct fsnotify_event *old_fsn,

			 struct fsnotify_event *new_fsn)

{

	pr_debug("%s: old=%p new=%p\n", __func__, old, new);

	struct fanotify_event_info *old, *new;



	if (old->to_tell == new->to_tell &&

	    old->data_type == new->data_type &&

	    old->tgid == new->tgid) {

		switch (old->data_type) {

		case (FSNOTIFY_EVENT_PATH):

#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	/* dont merge two permission events */

			if ((old->mask & FAN_ALL_PERM_EVENTS) &&

			    (new->mask & FAN_ALL_PERM_EVENTS))

	if ((old_fsn->mask & FAN_ALL_PERM_EVENTS) &&

	    (new_fsn->mask & FAN_ALL_PERM_EVENTS))

		return false;

#endif

			if ((old->path.mnt == new->path.mnt) &&

			    (old->path.dentry == new->path.dentry))

				return true;

			break;

		case (FSNOTIFY_EVENT_NONE):

	pr_debug("%s: old=%p new=%p\n", __func__, old_fsn, new_fsn);

	old = FANOTIFY_E(old_fsn);

	new = FANOTIFY_E(new_fsn);



	if (old_fsn->inode == new_fsn->inode && old->tgid == new->tgid &&

	    old->path.mnt == new->path.mnt &&

	    old->path.dentry == new->path.dentry)

		return true;

		default:

			BUG();

		};

	}

	return false;

}
@@ -41,59 +37,28 @@ static bool should_merge(struct fsnotify_event *old, struct fsnotify_event *new) 
static struct fsnotify_event *fanotify_merge(struct list_head *list,

					     struct fsnotify_event *event)

{

	struct fsnotify_event_holder *test_holder;

	struct fsnotify_event *test_event = NULL;

	struct fsnotify_event *new_event;

	struct fsnotify_event *test_event;

	bool do_merge = false;



	pr_debug("%s: list=%p event=%p\n", __func__, list, event);





	list_for_each_entry_reverse(test_holder, list, event_list) {

		if (should_merge(test_holder->event, event)) {

			test_event = test_holder->event;

	list_for_each_entry_reverse(test_event, list, list) {

		if (should_merge(test_event, event)) {

			do_merge = true;

			break;

		}

	}



	if (!test_event)

	if (!do_merge)

		return NULL;



	fsnotify_get_event(test_event);



	/* if they are exactly the same we are done */

	if (test_event->mask == event->mask)

		return test_event;



	/*

	 * if the refcnt == 2 this is the only queue

	 * for this event and so we can update the mask

	 * in place.

	 */

	if (atomic_read(&test_event->refcnt) == 2) {

	test_event->mask |= event->mask;

	return test_event;

}



	new_event = fsnotify_clone_event(test_event);



	/* done with test_event */

	fsnotify_put_event(test_event);



	/* couldn't allocate memory, merge was not possible */

	if (unlikely(!new_event))

		return ERR_PTR(-ENOMEM);



	/* build new event and replace it on the list */

	new_event->mask = (test_event->mask | event->mask);

	fsnotify_replace_event(test_holder, new_event);



	/* we hold a reference on new_event from clone_event */

	return new_event;

}



#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

static int fanotify_get_response_from_access(struct fsnotify_group *group,

					     struct fsnotify_event *event)

					     struct fanotify_event_info *event)

{

	int ret;
@@ -106,7 +71,6 @@ static int fanotify_get_response_from_access(struct fsnotify_group *group, 
		return 0;



	/* userspace responded, convert to something usable */

	spin_lock(&event->lock);

	switch (event->response) {

	case FAN_ALLOW:

		ret = 0;
@@ -116,7 +80,6 @@ static int fanotify_get_response_from_access(struct fsnotify_group *group, 
		ret = -EPERM;

	}

	event->response = 0;

	spin_unlock(&event->lock);



	pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,

		 group, event, ret);
@@ -125,48 +88,8 @@ static int fanotify_get_response_from_access(struct fsnotify_group *group, 
}

#endif



static int fanotify_handle_event(struct fsnotify_group *group,

				 struct fsnotify_mark *inode_mark,

				 struct fsnotify_mark *fanotify_mark,

				 struct fsnotify_event *event)

{

	int ret = 0;

	struct fsnotify_event *notify_event = NULL;



	BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);

	BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);

	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);

	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);

	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);

	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);

	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);

	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);

	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);

	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);



	pr_debug("%s: group=%p event=%p\n", __func__, group, event);



	notify_event = fsnotify_add_notify_event(group, event, NULL, fanotify_merge);

	if (IS_ERR(notify_event))

		return PTR_ERR(notify_event);



#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	if (event->mask & FAN_ALL_PERM_EVENTS) {

		/* if we merged we need to wait on the new event */

		if (notify_event)

			event = notify_event;

		ret = fanotify_get_response_from_access(group, event);

	}

#endif



	if (notify_event)

		fsnotify_put_event(notify_event);



	return ret;

}



static bool fanotify_should_send_event(struct fsnotify_group *group,

				       struct inode *to_tell,

				       struct inode *inode,

				       struct fsnotify_mark *inode_mark,

				       struct fsnotify_mark *vfsmnt_mark,

				       __u32 event_mask, void *data, int data_type)
@@ -174,8 +97,8 @@ static bool fanotify_should_send_event(struct fsnotify_group *group, 
	__u32 marks_mask, marks_ignored_mask;

	struct path *path = data;



	pr_debug("%s: group=%p to_tell=%p inode_mark=%p vfsmnt_mark=%p "

		 "mask=%x data=%p data_type=%d\n", __func__, group, to_tell,

	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);



	/* if we don't have enough info to send an event to userspace say no */
@@ -217,6 +140,70 @@ static bool fanotify_should_send_event(struct fsnotify_group *group, 
	return false;

}



static int fanotify_handle_event(struct fsnotify_group *group,

				 struct inode *inode,

				 struct fsnotify_mark *inode_mark,

				 struct fsnotify_mark *fanotify_mark,

				 u32 mask, void *data, int data_type,

				 const unsigned char *file_name)

{

	int ret = 0;

	struct fanotify_event_info *event;

	struct fsnotify_event *fsn_event;

	struct fsnotify_event *notify_fsn_event;



	BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);

	BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);

	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);

	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);

	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);

	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);

	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);

	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);

	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);

	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);



	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,

		 mask);



	event = kmem_cache_alloc(fanotify_event_cachep, GFP_KERNEL);

	if (unlikely(!event))

		return -ENOMEM;



	fsn_event = &event->fse;

	fsnotify_init_event(fsn_event, inode, mask);

	event->tgid = get_pid(task_tgid(current));

	if (data_type == FSNOTIFY_EVENT_PATH) {

		struct path *path = data;

		event->path = *path;

		path_get(&event->path);

	} else {

		event->path.mnt = NULL;

		event->path.dentry = NULL;

	}

#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	event->response = 0;

#endif



	notify_fsn_event = fsnotify_add_notify_event(group, fsn_event,

						     fanotify_merge);

	if (notify_fsn_event) {

		/* Our event wasn't used in the end. Free it. */

		fsnotify_destroy_event(group, fsn_event);

		if (IS_ERR(notify_fsn_event))

			return PTR_ERR(notify_fsn_event);

		/* We need to ask about a different events after a merge... */

		event = FANOTIFY_E(notify_fsn_event);

		fsn_event = notify_fsn_event;

	}



#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	if (fsn_event->mask & FAN_ALL_PERM_EVENTS)

		ret = fanotify_get_response_from_access(group, event);

#endif

	return ret;

}



static void fanotify_free_group_priv(struct fsnotify_group *group)

{

	struct user_struct *user;
@@ -226,10 +213,20 @@ static void fanotify_free_group_priv(struct fsnotify_group *group) 
	free_uid(user);

}



static void fanotify_free_event(struct fsnotify_event *fsn_event)

{

	struct fanotify_event_info *event;



	event = FANOTIFY_E(fsn_event);

	path_put(&event->path);

	put_pid(event->tgid);

	kmem_cache_free(fanotify_event_cachep, 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_priv = NULL,

	.free_event = fanotify_free_event,

	.freeing_mark = NULL,

};

fs/notify/fanotify/fanotify.h

0 → 100644
+23 −0
Original line number Diff line number Diff line 
#include <linux/fsnotify_backend.h>

#include <linux/path.h>

#include <linux/slab.h>



extern struct kmem_cache *fanotify_event_cachep;



struct fanotify_event_info {

	struct fsnotify_event fse;

	/*

	 * We hold ref to this path so it may be dereferenced at any point

	 * during this object's lifetime

	 */

	struct path path;

	struct pid *tgid;

#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	u32 response;	/* userspace answer to question */

#endif

};



static inline struct fanotify_event_info *FANOTIFY_E(struct fsnotify_event *fse)

{

	return container_of(fse, struct fanotify_event_info, fse);

}

fs/notify/fanotify/fanotify_user.c

+20 −21
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 


#include "../../mount.h"

#include "../fdinfo.h"

#include "fanotify.h"



#define FANOTIFY_DEFAULT_MAX_EVENTS	16384

#define FANOTIFY_DEFAULT_MAX_MARKS	8192
@@ -28,11 +29,12 @@ extern const struct fsnotify_ops fanotify_fsnotify_ops; 


static struct kmem_cache *fanotify_mark_cache __read_mostly;

static struct kmem_cache *fanotify_response_event_cache __read_mostly;

struct kmem_cache *fanotify_event_cachep __read_mostly;



struct fanotify_response_event {

	struct list_head list;

	__s32 fd;

	struct fsnotify_event *event;

	struct fanotify_event_info *event;

};



/*
@@ -61,7 +63,7 @@ static struct fsnotify_event *get_one_event(struct fsnotify_group *group, 
}



static int create_fd(struct fsnotify_group *group,

			struct fsnotify_event *event,

		     struct fanotify_event_info *event,

		     struct file **file)

{

	int client_fd;
@@ -73,12 +75,6 @@ static int create_fd(struct fsnotify_group *group, 
	if (client_fd < 0)

		return client_fd;



	if (event->data_type != FSNOTIFY_EVENT_PATH) {

		WARN_ON(1);

		put_unused_fd(client_fd);

		return -EINVAL;

	}



	/*

	 * we need a new file handle for the userspace program so it can read even if it was

	 * originally opened O_WRONLY.
@@ -110,22 +106,24 @@ static int create_fd(struct fsnotify_group *group, 


static int fill_event_metadata(struct fsnotify_group *group,

			       struct fanotify_event_metadata *metadata,

				   struct fsnotify_event *event,

			       struct fsnotify_event *fsn_event,

			       struct file **file)

{

	int ret = 0;

	struct fanotify_event_info *event;



	pr_debug("%s: group=%p metadata=%p event=%p\n", __func__,

		 group, metadata, event);

		 group, metadata, fsn_event);



	*file = NULL;

	event = container_of(fsn_event, struct fanotify_event_info, fse);

	metadata->event_len = FAN_EVENT_METADATA_LEN;

	metadata->metadata_len = FAN_EVENT_METADATA_LEN;

	metadata->vers = FANOTIFY_METADATA_VERSION;

	metadata->reserved = 0;

	metadata->mask = event->mask & FAN_ALL_OUTGOING_EVENTS;

	metadata->mask = fsn_event->mask & FAN_ALL_OUTGOING_EVENTS;

	metadata->pid = pid_vnr(event->tgid);

	if (unlikely(event->mask & FAN_Q_OVERFLOW))

	if (unlikely(fsn_event->mask & FAN_Q_OVERFLOW))

		metadata->fd = FAN_NOFD;

	else {

		metadata->fd = create_fd(group, event, file);
@@ -209,7 +207,7 @@ static int prepare_for_access_response(struct fsnotify_group *group, 
	if (!re)

		return -ENOMEM;



	re->event = event;

	re->event = FANOTIFY_E(event);

	re->fd = fd;



	mutex_lock(&group->fanotify_data.access_mutex);
@@ -217,7 +215,7 @@ static int prepare_for_access_response(struct fsnotify_group *group, 
	if (atomic_read(&group->fanotify_data.bypass_perm)) {

		mutex_unlock(&group->fanotify_data.access_mutex);

		kmem_cache_free(fanotify_response_event_cache, re);

		event->response = FAN_ALLOW;

		FANOTIFY_E(event)->response = FAN_ALLOW;

		return 0;

	}
@@ -273,7 +271,7 @@ out_close_fd: 
out:

#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS

	if (event->mask & FAN_ALL_PERM_EVENTS) {

		event->response = FAN_DENY;

		FANOTIFY_E(event)->response = FAN_DENY;

		wake_up(&group->fanotify_data.access_waitq);

	}

#endif
@@ -321,7 +319,7 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, 
			if (IS_ERR(kevent))

				break;

			ret = copy_event_to_user(group, kevent, buf);

			fsnotify_put_event(kevent);

			fsnotify_destroy_event(group, kevent);

			if (ret < 0)

				break;

			buf += ret;
@@ -409,7 +407,7 @@ static int fanotify_release(struct inode *ignored, struct file *file) 
static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

{

	struct fsnotify_group *group;

	struct fsnotify_event_holder *holder;

	struct fsnotify_event *fsn_event;

	void __user *p;

	int ret = -ENOTTY;

	size_t send_len = 0;
@@ -421,7 +419,7 @@ static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long ar 
	switch (cmd) {

	case FIONREAD:

		mutex_lock(&group->notification_mutex);

		list_for_each_entry(holder, &group->notification_list, event_list)

		list_for_each_entry(fsn_event, &group->notification_list, list)

			send_len += FAN_EVENT_METADATA_LEN;

		mutex_unlock(&group->notification_mutex);

		ret = put_user(send_len, (int __user *) p);
@@ -906,6 +904,7 @@ static int __init fanotify_user_setup(void) 
	fanotify_mark_cache = KMEM_CACHE(fsnotify_mark, SLAB_PANIC);

	fanotify_response_event_cache = KMEM_CACHE(fanotify_response_event,

						   SLAB_PANIC);

	fanotify_event_cachep = KMEM_CACHE(fanotify_event_info, SLAB_PANIC);



	return 0;

}

fs/notify/fsnotify.c

+12 −25
Original line number Diff line number Diff line
@@ -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;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE