From: Jan Kara <jack@suse.cz>
To: boyd yang <boyd.yang@gmail.com>
Cc: Josef Bacik <josef@redhat.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
eparis@redhat.com, xiyou.wangcong@gmail.com
Subject: Re: [PATCH] fanotify: to differ file access event from different threads
Date: Thu, 24 Nov 2011 15:58:22 +0100 [thread overview]
Message-ID: <20111124145822.GC22640@quack.suse.cz> (raw)
In-Reply-To: <CAE8kqZ+GV3CoxWMV=4GXgc6YYCcnAk_jnBxzwRubgccZL8VstA@mail.gmail.com>
On Thu 17-11-11 18:21:26, boyd yang wrote:
> How can this patch get merged?
> Who is responsible for this?
Eric Paris is responsible for merging this patch. I see you have CCed him
so that should be OK. Can you maybe resend the patch in a separate email
again? I see your last submission was included in a reply to email together
with other text etc. which makes merging it using our standard tools
harder...
Honza
> We are developing some real-time antivirus software, the fanotify can make
> it work without dirver/kenel-modues( like RedirFS).
> But the bug costs a lot trouble.
> If we used the patch, our software works great!
>
> I think other antivirus or file-motoring softwares are influnced by the bug
> too.
>
>
> On Fri, Oct 14, 2011 at 2:56 PM, boyd yang <boyd.yang@gmail.com> wrote:
> >
> > Thanks, I updated the patch.
> > Now it passed checkpatch.pl.
> >
> > I used the flag you mentioned.
> >
> > 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!\n");
> > + } 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
> >
> >
> >
> >
> >
> > On Thu, Oct 13, 2011 at 9:38 PM, Josef Bacik <josef@redhat.com> wrote:
> > > On Thu, Oct 13, 2011 at 04:56:43PM +0800, boyd yang wrote:
> > >> This patch fixes a hang problem of Eric Paris's fs Notification/fanotify.
> > >>
> > >> Fanotify brings a way to intercept file access events.
> > >> When multiple threadsiterate 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.
> > >>
> > >
> > > You need to run this through checkpatch.pl, you have a ton of formatting
> > > problems. Also your email client seems to have word-wrapped parts of this, so
> > > use a email client that doesn't word wrap.
> > >
> > >> =============================================================
> > >>
> > >> 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-10
> > >> 12:28:23.276847000 +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,19 @@
> > >> 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(fsnotify_isoverflow(event))
> > >> + {
> > >> + pr_debug("fanotify overflow!\n");
> > >> + }
> > >> + 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);
> > >> + }
> > >> }
> > >
> > > The overflow event should only have FS_Q_OVERFLOW set in it's mask right? So
> > > why is this test needed at all? Thanks,
> > >
> > > Josef
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
next prev parent reply other threads:[~2011-11-24 14:58 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-13 8:56 [PATCH] fanotify: to differ file access event from different threads boyd yang
2011-10-13 13:27 ` boyd yang
2011-10-13 14:35 ` Américo Wang
2011-10-13 13:38 ` Josef Bacik
2011-10-14 6:56 ` boyd yang
2011-10-14 6:56 ` boyd yang
2011-11-17 10:21 ` boyd yang
2011-11-24 14:58 ` Jan Kara [this message]
2011-12-06 1:24 ` boyd yang
-- strict thread matches above, loose matches on Subject: below --
2011-12-06 1:23 boyd yang
2011-12-10 16:25 ` Lino Sanfilippo
2013-02-11 19:49 ` Mihai Donțu
2011-10-10 6:45 boyd yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111124145822.GC22640@quack.suse.cz \
--to=jack@suse.cz \
--cc=boyd.yang@gmail.com \
--cc=eparis@redhat.com \
--cc=josef@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.