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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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@xxxxxxxxx>
> >
> > 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux