Re: fsnotify_mark_srcu wtf?

[Amir Goldstein]

On Thu, Nov 3, 2016 at 12:09 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> We've got a report where a fanotify daemon that implements permission checks
> screws up and doesn't send a reply.  This then causes widespread hangs due to
> fsnotify_mark_srcu read side lock being held and thus causing synchronize_srcu()
> called from e.g. inotify_release()-> fsnotify_destroy_group()->
> fsnotify_mark_destroy_list() to block.
>
> Below program demonstrates the issue.  It should output a single line:
>
> close(inotify_fd): success
>
> Instead it outputs nothing, which means that close(inotify_fd) got blocked by
> the waiting permission event.
>
> Wouldn't making the srcu per-group fix this?  Would that be too expensive?
>

IIUC, the purpose of fsnotify_mark_srcu is to protect the inode and vfsmount
mark lists, which are used to iterate the groups to send events to.
So you cannot make it per group as far as I can tell.

OTOH, specifically, for fanotify, once you can passed
fanotify_should_send_event(),
there is no need to keep a reference to the mark, so it may be safely destroyed,
you only need a reference to the group.

I tested this patch on top of my fanotify super block watch development branch
and it seems to fix the problem you reported, but I am not savvy enough with
srcu to say that this is correct.
Jan, what do you think?