Re: [PATCH RFC] nfsd: avoid recursive locking through fsnotify

[Amir Goldstein <amir73il@gmail.com>]

> > I think I might have misunderstood you and you meant that the
> > SINGLE_DEPTH_NESTING subcalls should be eliminated and then
> > we are left with two lock classes.
> > Correct?
>
> Yeah, at least at this point I don't see a good reason for using
> SINGLE_DEPTH_NESTING lockdep annotation. In my opinion it has just a
> potential of silencing reports of real locking problems. So removing it and
> seeing what complains would be IMO a way to go.
>

Agreed. In fact, the reason it was added is based on a misunderstanding
of mutex_lock_nested():

Commit 6960b0d909cd ("fsnotify: change locking order") changed some
    of the mark_mutex locks in direct reclaim path to use:
      mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING);

    This change is explained:
     "...It uses nested locking to avoid deadlock in case we do the final
      iput() on an inode which still holds marks and thus would take the
      mutex again when calling fsnotify_inode_delete() in destroy_inode()."

Pushed the fix along with conversion of all backends to fsnotify_group_lock()
to fan_evictable branch, which I am still testing.

It's worth noting that I found dnotify to be not fs reclaim safe, because
dnotify_flush() is called from any filp_close() (problem is explained in the
commit message), which contradicts my earlier argument that legacy
backends "must be deadlock safe or we would have gotten deadlock
reports by now".

So yeh, eventually, we may set all legacy backends NOFS, but I would like
to try and exempt inotify and audit for now to reduce the chance of regressions.

Thanks,
Amir.