All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>, Neil Brown <neilb@suse.de>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	"J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/7] nfsd: only check RPC_SIGNALLED() when restarting rpc_task
Date: Sat, 8 Feb 2025 11:59:28 -0500	[thread overview]
Message-ID: <610bc63d-1ce3-40d1-aa42-bfb22dc26ac4@oracle.com> (raw)
In-Reply-To: <20250207-nfsd-6-14-v5-3-f3b54fb60dc0@kernel.org>

On 2/7/25 4:53 PM, Jeff Layton wrote:
> nfsd4_cb_sequence_done() currently checks RPC_SIGNALLED() when
> processing the compound and releasing the slot. If RPC_SIGNALLED()
> returns true, then that means that the client is going to be torn down.
> 
> Don't check RPC_SIGNALLED() after processing a successful reply.
> Instead, only check that before restarting the rpc_task. If that returns
> true, then requeue the callback.

This might seem like a nit, but this paragraph didn't make sense to me
at all until I changed s/only check that before/check that only before/.


> Also, handle rpc_restart_call() and rpc_restart_call_prepare() failures
> correctly, by requeueing the callback.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4callback.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index bb5356e8713a8840bb714859618ff88130825efd..1e601075fac02bd5f01ff89d9252ab23d08c4fd9 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1385,8 +1385,8 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
>  		goto requeue;
>  	case -NFS4ERR_DELAY:
>  		cb->cb_seq_status = 1;
> -		if (!rpc_restart_call(task))
> -			goto out;
> +		if (RPC_SIGNALLED(task) || !rpc_restart_call(task))
> +			goto requeue;
>  		rpc_delay(task, 2 * HZ);
>  		return false;
>  	case -NFS4ERR_BADSLOT:
> @@ -1402,14 +1402,16 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
>  	}
>  	trace_nfsd_cb_free_slot(task, cb);
>  	nfsd41_cb_release_slot(cb);
> -
> -	if (RPC_SIGNALLED(task))
> -		goto requeue;
> -out:
>  	return ret;
>  retry_nowait:
> -	rpc_restart_call_prepare(task);
> -	goto out;
> +	/*
> +	 * RPC_SIGNALLED() means that the rpc_client is being torn down and
> +	 * (possibly) recreated. Requeue the call in that case.
> +	 */
> +	if (!RPC_SIGNALLED(task)) {
> +		if (rpc_restart_call_prepare(task))
> +			return false;
> +	}
>  requeue:
>  	nfsd41_cb_release_slot(cb);
>  	if (!test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags)) {
> 


-- 
Chuck Lever

  reply	other threads:[~2025-02-08 16:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-07 21:53 [PATCH v5 0/7] nfsd: CB_SEQUENCE error handling fixes and cleanups Jeff Layton
2025-02-07 21:53 ` [PATCH v5 1/7] nfsd: prepare nfsd4_cb_sequence_done() for error handling rework Jeff Layton
2025-02-07 21:53 ` [PATCH v5 2/7] nfsd: always release slot when requeueing callback Jeff Layton
2025-02-08 16:57   ` Chuck Lever
2025-02-08 17:55     ` Jeff Layton
2025-02-07 21:53 ` [PATCH v5 3/7] nfsd: only check RPC_SIGNALLED() when restarting rpc_task Jeff Layton
2025-02-08 16:59   ` Chuck Lever [this message]
2025-02-07 21:53 ` [PATCH v5 4/7] nfsd: when CB_SEQUENCE gets ESERVERFAULT don't increment seq_nr Jeff Layton
2025-02-08 17:13   ` Chuck Lever
2025-02-07 21:53 ` [PATCH v5 5/7] nfsd: handle CB_SEQUENCE NFS4ERR_BADSLOT better Jeff Layton
2025-02-07 21:53 ` [PATCH v5 6/7] nfsd: handle CB_SEQUENCE NFS4ERR_SEQ_MISORDERED error better Jeff Layton
2025-02-08 17:01   ` Chuck Lever
2025-02-08 18:02     ` Jeff Layton
2025-02-08 18:40       ` Tom Talpey
2025-02-08 19:08         ` Jeff Layton
2025-02-08 19:18           ` Tom Talpey
2025-02-08 20:45             ` Jeff Layton
2025-02-08 21:07               ` Chuck Lever
2025-02-09  1:24                 ` Tom Talpey
2025-02-09  2:14                   ` Jeff Layton
2025-02-09 16:26                     ` Tom Talpey
2025-02-09 16:51                       ` Jeff Layton
2025-02-09 16:58                         ` Tom Talpey
2025-02-09 17:05                           ` Jeff Layton
2025-02-09 18:52                             ` Tom Talpey
2025-02-07 21:53 ` [PATCH v5 7/7] nfsd: lift NFSv4.0 handling out of nfsd4_cb_sequence_done() Jeff Layton
2025-02-08 17:05   ` Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=610bc63d-1ce3-40d1-aa42-bfb22dc26ac4@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=bfields@fieldses.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.