On Fri, 15 Jul 2022, Jeff Layton wrote:
> This is a respin of the patchset that I sent earlier today. I hit a
> deadlock with that one because of the ambiguous locking.
>
> This series is based on top of Neil's set entitled:
>
> [PATCH 0/8] NFSD: clean up locking.
>
> His patchset makes the locking in the nfsd4_open codepath much more
> consistent, and this becomes a lot simpler to fix. Without that set
> however, the state of the parent's i_rwsem is unclear after nfsd_lookup
> is called, and I don't see a way to determine it reliably.
I haven't examined these patch very closely, but a few initial thoughts
are:
1/ Before my series, you can unambiguously tell if i_rwsem is held by
checking fhp->fh_locked. In fact, just call "fh_lock()", and you can
then be sure the fh is locked, whether or not it was locked before
however...
2/ Do we really need to lock the parent? If a rename or unlink happens
after the lease was taken, the lease will be broken. So
take lease.
repeat lookup (locklessly)
Check if lease has been broken
Should provide all you need.
You don't *need* to lock the directory to open an existing file and
with my pending parallel-updates patch set, you only need a shared
lock on the directory to create a file. So I'd rather not be locking
the directory at all to get a delegation
3/ When you vet the name you only do a lookup_one_len(), while
nfsd_lookup_dentry() also calls nfsd_cross_mnt() as it is possible
for a file to be mounted on.
That means that if I did bind mount one file over another and export
over NFSD, the file will never offer a delegation.
This is a minor point, but I think it would be best to be as correct
and consistent as possible.
Thanks for working on this!
NeilBrown
>
> Jeff Layton (2):
> nfsd: drop fh argument from alloc_init_deleg
> nfsd: vet the opened dentry after setting a delegation
>
> fs/nfsd/nfs4state.c | 54 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 45 insertions(+), 9 deletions(-)
>
> --
> 2.36.1
>
>
