Re: [PATCH v2 2/2] lockd: while grace prefer to fail with nlm_lck_denied_grace_period

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 13 Aug 2025, Olga Kornievskaia wrote:
> When nfsd is in grace and receives an NLM LOCK request which turns
> out to have a conflicting delegation, return that the server is in
> grace.
> 
> Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> ---
>  fs/lockd/svc4proc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> index 109e5caae8c7..7ac4af5c9875 100644
> --- a/fs/lockd/svc4proc.c
> +++ b/fs/lockd/svc4proc.c
> @@ -141,8 +141,19 @@ __nlm4svc_proc_lock(struct svc_rqst *rqstp, struct nlm_res *resp)
>  	resp->cookie = argp->cookie;
>  
>  	/* Obtain client and file */
> -	if ((resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file)))
> -		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
> +	resp->status = nlm4svc_retrieve_args(rqstp, argp, &host, &file);
> +	switch (resp->status) {
> +	case 0:
> +		break;
> +	case nlm_drop_reply:
> +		if (locks_in_grace(SVC_NET(rqstp))) {
> +			resp->status = nlm_lck_denied_grace_period;

I think this is wrong.  If the lock request has the "reclaim" flag set,
then nlm_lck_denied_grace_period is not a meaningful error.
nlm4svc_retrieve_args() returns nlm_drop_reply when there is a delay
getting a response to an upcall to mountd.  For NLM the request really
must be dropped.

At the very least we need to guard against the reclaim flag being set in
the above test.  But I would much rather a more clear distinction were
made between "drop because of a conflicting delegation" and "drop
because of a delay getting upcall response".
Maybe a new "nlm_conflicting_delegtion" return from ->fopen which nlm4
(and ideally nlm2) handles appropriately.

NeilBrown


> +			return rpc_success;
> +		}
> +		return nlm_drop_reply;
> +	default:
> +		return rpc_success;
> +	}
>  
>  	/* Now try to lock the file */
>  	resp->status = nlmsvc_lock(rqstp, file, host, &argp->lock,
> -- 
> 2.47.1
> 
> 






[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux