On Mon, Apr 22 2019, Jeff Layton wrote: > After a blocked nfsd file_lock request is deleted, knfsd will send a > callback to the client and then free the request. Commit 16306a61d3b7 > ("fs/locks: always delete_block after waiting.") changed it such that > locks_delete_block is always called on a request after it is awoken, > but that patch missed fixing up blocked nfsd request handling. > > Call locks_delete_block on the block to wake up any locks still blocked > on the nfsd lock request before freeing it. Some of its callers already > do this however, so just remove those calls. > > URL: https://bugzilla.kernel.org/show_bug.cgi?id=203363 > Fixes: 16306a61d3b7 ("fs/locks: always delete_block after waiting.") > Reported-by: Slawomir Pryczek <slawek1211@xxxxxxxxx> > Cc: Neil Brown <neilb@xxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > fs/nfsd/nfs4state.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 6a45fb00c5fc..e87e15df2044 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -265,6 +265,7 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh, > static void > free_blocked_lock(struct nfsd4_blocked_lock *nbl) > { > + locks_delete_block(&nbl->nbl_lock); > locks_release_private(&nbl->nbl_lock); Thanks for tracking this down. An implication of this bug and fix is that we need to be particularly careful to make sure locks_delete_block() is called on all relevant paths. Can we make that easier? My first thought was to include the call in locks_release_private, but lockd calls the two quite separately and it certainly seems appropriate that locks_delete_block should be called asap, but locks_release_private() can be delayed. Also cifs calls locks_delete_block, but never calls locks_release_private, so it wouldn't help there. Looking at cifs, I think there is a call missing there too. cifs_posix_lock_set() *doesn't* always call locks_delete_block() after waiting. In particular, if ->can_cache_brlcks becomes true while waiting then I don't think the behaviour is right.... though I'm not sure it is right for other reasons. It looks like the return value should be 1 in that case, but it'll be zero. But back to my question about making it easier, move the BUG_ON() calls from locks_free_lock() into locks_release_private(). ?? Thanks, NeilBrown > kfree(nbl); > } > @@ -293,7 +294,6 @@ remove_blocked_locks(struct nfs4_lockowner *lo) > nbl = list_first_entry(&reaplist, struct nfsd4_blocked_lock, > nbl_lru); > list_del_init(&nbl->nbl_lru); > - locks_delete_block(&nbl->nbl_lock); > free_blocked_lock(nbl); > } > } > @@ -4863,7 +4863,6 @@ nfs4_laundromat(struct nfsd_net *nn) > nbl = list_first_entry(&reaplist, > struct nfsd4_blocked_lock, nbl_lru); > list_del_init(&nbl->nbl_lru); > - locks_delete_block(&nbl->nbl_lock); > free_blocked_lock(nbl); > } > out: > -- > 2.20.1
Attachment:
signature.asc
Description: PGP signature