On 2/7/25 4:53 PM, Jeff Layton wrote:
> It's a bit strange to call nfsd4_cb_sequence_done() on a callback with no
> CB_SEQUENCE. Lift the handling of restarting a call into a new helper,
> and move the handling of NFSv4.0 into nfsd4_cb_done().
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfsd/nfs4callback.c | 53 ++++++++++++++++++++++++++------------------------
> 1 file changed, 28 insertions(+), 25 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index d6e3e8bb2efabadda9f922318880e12e1cb2c23f..a4427e2f6182415755b646dba1a1ef4acddc0709 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1328,28 +1328,23 @@ static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
> rpc_call_start(task);
> }
>
> -/* Returns true if CB_COMPOUND processing should continue */
> -static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback *cb)
> +static void requeue_callback(struct rpc_task *task, struct nfsd4_callback *cb)
Nit: requeue_callback => nfsd4_requeue_cb().
Once the nfsd41_cb_release_slot() call is gone from this helper, it
can easily be placed just after nfsd4_queue_cb(), which seems like
more legible code organization.
Just small nits this time. v6 ought to be ready for me to pull.
> {
> struct nfs4_client *clp = cb->cb_clp;
> - struct nfsd4_session *session = clp->cl_cb_session;
> - bool ret = false;
> -
> - if (!clp->cl_minorversion) {
> - /*
> - * If the backchannel connection was shut down while this
> - * task was queued, we need to resubmit it after setting up
> - * a new backchannel connection.
> - *
> - * Note that if we lost our callback connection permanently
> - * the submission code will error out, so we don't need to
> - * handle that case here.
> - */
> - if (RPC_SIGNALLED(task))
> - goto requeue;
>
> - return true;
> + nfsd41_cb_release_slot(cb);
> + if (!test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags)) {
> + trace_nfsd_cb_restart(clp, cb);
> + task->tk_status = 0;
> + cb->cb_need_restart = true;
> }
> +}
> +
> +/* Returns true if CB_COMPOUND processing should continue */
> +static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback *cb)
> +{
> + struct nfsd4_session *session = cb->cb_clp->cl_cb_session;
> + bool ret = false;
>
> if (cb->cb_held_slot < 0)
> goto requeue;
> @@ -1429,12 +1424,7 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
> return false;
> }
> requeue:
> - nfsd41_cb_release_slot(cb);
> - if (!test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags)) {
> - trace_nfsd_cb_restart(clp, cb);
> - task->tk_status = 0;
> - cb->cb_need_restart = true;
> - }
> + requeue_callback(task, cb);
> return false;
> }
>
> @@ -1445,8 +1435,21 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
>
> trace_nfsd_cb_rpc_done(clp);
>
> - if (!nfsd4_cb_sequence_done(task, cb))
> + if (!clp->cl_minorversion) {
> + /*
> + * If the backchannel connection was shut down while this
> + * task was queued, we need to resubmit it after setting up
> + * a new backchannel connection.
> + *
> + * Note that if we lost our callback connection permanently
> + * the submission code will error out, so we don't need to
> + * handle that case here.
> + */
> + if (RPC_SIGNALLED(task))
> + requeue_callback(task, cb);
> + } else if (!nfsd4_cb_sequence_done(task, cb)) {
> return;
> + }
>
> if (cb->cb_status) {
> WARN_ONCE(task->tk_status,
>
--
Chuck Lever
