Re: [PATCH] NFS: Return a delegation the client fails to record

Hi Chuck,

On Thu, Jul 2, 2026, at 10:51 AM, Chuck Lever wrote:
> On Wed, Jun 17, 2026, at 8:52 AM, Chuck Lever wrote:
>> When an NFS server grants a delegation in an OPEN reply,
>> nfs_inode_set_delegation() records it on the client. However, three
>> of its error flows return without sending DELEGRETURN.
>>
>> A delegation can be relinquished only by DELEGRETURN (RFC 8881
>> Section 20.2.4), so dropping one silently leaves the server believing
>> the client still holds it. If the server happens to recall that
>> delegation, the client answers CB_RECALL with NFS4ERR_BADHANDLE
>> because it has no record of the stateid. The server revokes the
>> delegation and moves it onto its cl_revoked list, because the client
>> never sends the FREE_STATEID that would drain it. Every subsequent
>> SEQUENCE reply then carries SEQ4_STATUS_RECALLABLE_STATE_REVOKED,
>> and the client's state manager loops issuing TEST_STATEID across its
>> delegations without ever clearing the condition.
>>
>> The window is easy to reach now that a server offers a write
>> delegation on any write OPEN: a delegation recalled for one opener
>> races a re-open that the server answers with a fresh write
>> delegation.
>>
>> Instead of dropping it, hand the delegation back during these error
>> flows.
>>
>> Fixes: ade04647dd56 ("NFSv4: Ensure we honour NFS_DELEGATION_RETURNING 
>> in nfs_inode_set_delegation()")
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>  fs/nfs/delegation.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
>> index 122fb3f14ffb..cb579f8c55ce 100644
>> --- a/fs/nfs/delegation.c
>> +++ b/fs/nfs/delegation.c
>> @@ -440,11 +440,14 @@ int nfs_inode_set_delegation(struct inode *inode, 
>> const struct cred *cred,
>>  	struct nfs_inode *nfsi = NFS_I(inode);
>>  	struct nfs_delegation *delegation, *old_delegation;
>>  	struct nfs_delegation *freeme = NULL;
>> +	bool orphaned = false;
>>  	int status = 0;
>> 
>>  	delegation = kmalloc_obj(*delegation, GFP_KERNEL_ACCOUNT);
>> -	if (delegation == NULL)
>> +	if (delegation == NULL) {
>> +		nfs4_proc_delegreturn(inode, cred, stateid, NULL, 0);
>>  		return -ENOMEM;
>> +	}
>>  	nfs4_stateid_copy(&delegation->stateid, stateid);
>>  	refcount_set(&delegation->refcount, 1);
>>  	delegation->type = type;
>> @@ -493,11 +496,15 @@ int nfs_inode_set_delegation(struct inode *inode, 
>> const struct cred *cred,
>>  			goto out;
>>  		}
>>  		if (test_and_set_bit(NFS_DELEGATION_RETURNING,
>> -					&old_delegation->flags))
>> +					&old_delegation->flags)) {
>> +			orphaned = true;
>>  			goto out;
>> +		}
>>  	}
>> -	if (!nfs_detach_delegations_locked(nfsi, old_delegation, clp))
>> +	if (!nfs_detach_delegations_locked(nfsi, old_delegation, clp)) {
>> +		orphaned = true;
>>  		goto out;
>> +	}
>>  	freeme = old_delegation;
>>  add_new:
>>  	/*
>> @@ -532,8 +539,11 @@ int nfs_inode_set_delegation(struct inode *inode, 
>> const struct cred *cred,
>>  		nfs_update_delegated_mtime(inode);
>>  out:
>>  	spin_unlock(&clp->cl_lock);
>> -	if (delegation != NULL)
>> +	if (delegation != NULL) {
>> +		if (orphaned)
>> +			nfs_do_return_delegation(inode, delegation, 0);
>>  		__nfs_free_delegation(delegation);
>> +	}
>>  	if (freeme != NULL) {
>>  		nfs_do_return_delegation(inode, freeme, 0);
>>  		nfs_mark_delegation_revoked(server, freeme);
>> -- 
>> 2.54.0
>
> Friendly bump! What is the disposition of this submission?

I have it in a testing branch that I haven't pushed out yet. I'm planning
to include it in the next round of bugfixes that I send out.

Anna

>
>
> -- 
> Chuck Lever