[PATCH] fanotify: to differ file access event from different threads

December 05th, 2011 - 08:30 pm ET by boyd yang | Report spam
fanotify: to differ file access event from different threads
When fanotify is monitoring the whole mount point "/", and multiple
threads iterate the same direcotry, some thread will hang.
This patch let fanotify to differ access events from different
threads, prevent fanotify from merging access events from different
threads.
It also hide overflow events to reach user space.
Signed-off-by: Boyd Yang <boyd.yang@gmail.com>

diff -r -u linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c
linux-3.1-rc4/fs/notify/fanotify/fanotify.c
linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c 2011-08-29
12:16:01.000000000 +0800
+++ linux-3.1-rc4/fs/notify/fanotify/fanotify.c 2011-10-14
14:17:53.055958000 +0800
@@ -15,7 +15,8 @@

if (old->to_tell == new->to_tell &&
old->data_type == new->data_type &&
- old->tgid == new->tgid) {
+ old->tgid == new->tgid &&
+ old->pid == new->pid) {
switch (old->data_type) {
case (FSNOTIFY_EVENT_PATH):
if ((old->path.mnt == new->path.mnt) &&
@@ -144,11 +145,16 @@
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);
+ /*if overflow, do not wait for response*/
+ if (event->mask&FS_Q_OVERFLOW) {
+ pr_debug("fanotify overflow!");
+ } else {
+ 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

diff -r -u linux-3.1-rc4_orig/fs/notify/notification.c
linux-3.1-rc4/fs/notify/notification.c
linux-3.1-rc4_orig/fs/notify/notification.c 2011-08-29
12:16:01.000000000 +0800
+++ linux-3.1-rc4/fs/notify/notification.c 2011-10-14 13:52:36.946608000 +0800
@@ -95,6 +95,7 @@
BUG_ON(!list_empty(&event->private_data_list));

kfree(event->file_name);
+ put_pid(event->pid);
put_pid(event->tgid);
kmem_cache_free(fsnotify_event_cachep, event);
}
@@ -374,6 +375,7 @@
return NULL;
}
}
+ event->pid = get_pid(old_event->pid);
event->tgid = get_pid(old_event->tgid);
if (event->data_type == FSNOTIFY_EVENT_PATH)
path_get(&event->path);
@@ -417,6 +419,7 @@
event->name_len = strlen(event->file_name);
}

+ event->pid = get_pid(task_pid(current));
event->tgid = get_pid(task_tgid(current));
event->sync_cookie = cookie;
event->to_tell = to_tell;
diff -r -u linux-3.1-rc4_orig/include/linux/fsnotify_backend.h
linux-3.1-rc4/include/linux/fsnotify_backend.h
linux-3.1-rc4_orig/include/linux/fsnotify_backend.h 2011-08-29
12:16:01.000000000 +0800
+++ linux-3.1-rc4/include/linux/fsnotify_backend.h 2011-10-14
13:51:50.380168000 +0800
@@ -238,6 +238,7 @@
u32 sync_cookie; /* used to corrolate events, namely inotify mv events */
const unsigned char *file_name;
size_t name_len;
+ struct pid *pid;
struct pid *tgid;

#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
email Follow the discussionReplies 1 replyReplies Make a reply

Replies

#1 Lino Sanfilippo
December 10th, 2011 - 11:30 am ET | Report spam
On Tue, Dec 06, 2011 at 09:23:25AM +0800, boyd yang wrote:
fanotify: to differ file access event from different threads
When fanotify is monitoring the whole mount point "/", and multiple
threads iterate the same direcotry, some thread will hang.
This patch let fanotify to differ access events from different
threads, prevent fanotify from merging access events from different
threads.



I could reproduce the problem you described. Actually it may occur each time
when more than one thread of a process is waiting in fanotify_get_response()
at the same time. This is since we are not prepared to wake up more than one
waiter: We do

wait_event(group->fanotify_data.access_waitq, event->response ||
atomic_read(&group->fanotify_data.bypass_perm));

and after that
event->response = 0;

which is the reason that even if we woke up other waiters on the same waitqueue
they may see event->response being already 0, go back to sleep and then possibly
hang forever.

It also hide overflow events to reach user space.



This is not part of this patch and also should not be, since overflow
events are supposted to reach user space.

- 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);
+ /*if overflow, do not wait for response*/
+ if (event->mask&FS_Q_OVERFLOW) {
+ pr_debug("fanotify overflow!");
+ } else {
+ 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);
+ }
}



What is this for? All you do is introduce a debug message for no real reason.
However your fix (avoid merging of events from the same process) seems to work.

Regards,
Lino
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Similar topics