On Tue 23-07-24 13:11:51, Christian Brauner wrote:
> On Tue, Jul 23, 2024 at 12:45:33PM GMT, Jan Kara wrote:
> > On Tue 23-07-24 09:59:54, David Howells wrote:
> > > When using cachefiles, lockdep may emit something similar to the circular
> > > locking dependency notice below. The problem appears to stem from the
> > > following:
> > >
> > > (1) Cachefiles manipulates xattrs on the files in its cache when called
> > > from ->writepages().
> > >
> > > (2) The setxattr() and removexattr() system call handlers get the name
> > > (and value) from userspace after taking the sb_writers lock, putting
> > > accesses of the vma->vm_lock and mm->mmap_lock inside of that.
> > >
> > > (3) The afs filesystem uses a per-inode lock to prevent multiple
> > > revalidation RPCs and in writeback vs truncate to prevent parallel
> > > operations from deadlocking against the server on one side and local
> > > page locks on the other.
> > >
> > > Fix this by moving the getting of the name and value in {get,remove}xattr()
> > > outside of the sb_writers lock. This also has the minor benefits that we
> > > don't need to reget these in the event of a retry and we never try to take
> > > the sb_writers lock in the event we can't pull the name and value into the
> > > kernel.
> >
> > Well, it seems like you are trying to get rid of the dependency
> > sb_writers->mmap_sem. But there are other places where this dependency is
>
> Independent of this issue, I think that moving the retrieval of name and
> value out of the lock is a good thing. The commit message might need to
> get reworded of course.
Oh, absolutely. I think the change itself makes sense, just it will not fix
what David hopes to fix :)
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
