On Fri, Sep 01 2017, J. Bruce Fields wrote: >> >> nfsd would need to find that delegation, prevent further delegations >> being handed out, and check that there aren't already conflicting >> delegations. If there are conflicts, recall them. Once there are no >> conflicting delegations, make the vfs_ request. > > The way that we currently serialize setting, unsetting, and breaking > delegations is by locks on the delegated inodes which aren't taken till > deeper in the vfs code. Do we? I can see nfs4_set_delegation adding a new delegation for a new client without entering the vfs at all if there is already a lease held. If there isn't a lease already, vfs_setlease() is called, which doesn't its own internal locking of course. Much the same applies to unsetting delegations. Breaking delegations involves nfsd_break_deleg_cb() which has a comment that it is called with i_lock held.... that seems to be used to be sure that it is safe to a reference to the delegation state id. Is that the only dependency on the vfs locking, or did I miss something? > > I guess you're suggesting adding a second mechanism to prevent > delegations being given out on the inode. We could add an atomic > counter taken by each nfsd breaker while it's in progress. Hrm. Something like that. We would also need to be able to look up an nfs4_file by inode (why *are* they hashed by file handle??) and add some wait queue somewhere so the breaker could wait for a delegation to be returned. My big-picture point is that any complexity created by NFSD's choice to merge delegations to multiple clients into a single vfs-level delegation should be handled by NFSD, and not imposed on the VFS. It certainly makes sense for the VFS to understand that certain operations are being performed by an fl_owner_t, and to allow delegations to that owner to remain. It doesn't make as much sense for the VFS to understand that there is a finer granularity of ownership than the one that it already supports. Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
