From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756518Ab1KXO60 (ORCPT ); Thu, 24 Nov 2011 09:58:26 -0500 Received: from cantor2.suse.de ([195.135.220.15]:35697 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753675Ab1KXO6Z (ORCPT ); Thu, 24 Nov 2011 09:58:25 -0500 Date: Thu, 24 Nov 2011 15:58:22 +0100 From: Jan Kara To: boyd yang Cc: Josef Bacik , 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 Message-ID: <20111124145822.GC22640@quack.suse.cz> References: <20111013133815.GC2310@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 > > > > 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 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 SUSE Labs, CR