On Sat, 11 May 2024 at 11:29, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Reorganize the code trivially to just have a separate success path,
> which simplifies the code (since 'd_delete_notify()' is only called in
> the success path anyway) and makes it trivial to just move the dentry
> shrinking outside the inode lock.
Bah.
I think this might need more work.
The *caller* of vfs_rmdir() also holds a lock, ie we have do_rmdir() doing
inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags);
...
error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode, dentry);
dput(dentry);
inode_unlock(path.dentry->d_inode);
so we have another level of locking going on, and my patch only moved
the dcache pruning outside the lock of the directory we're removing
(not outside the lock of the directory that contains the removed
directory).
And that outside lock is the much more important one, I bet.
So I still think this approach may be the right one, but that patch of
mine didn't go far enough.
Sadly, while do_rmdir() itself is trivial to fix up to do this, we
have several other users of vfs_rmdir() (ecryptfs, devtmpfs, overlayfs
in addition to nfsd and ksmbd), so the more complete patch would be
noticeably bigger.
My bad.
Linus
