linux-kernel - Re: [PATCH] sunrpc: make rpc_restart_call() and rpc_restart_call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f5bcf44a-822d-40bd-8e1c-19bafe9ea5d0@oracle.com>
Date: Wed, 5 Feb 2025 09:56:14 -0500
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>, Trond Myklebust <trondmy@...nel.org>,
        Anna Schumaker <anna@...nel.org>
Cc: Neil Brown <neilb@...e.de>, Olga Kornievskaia <okorniev@...hat.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
        linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sunrpc: make rpc_restart_call() and
 rpc_restart_call_prepare() void return

On 2/5/25 8:58 AM, Jeff Layton wrote:
> These functions always return 1. Make them void return and fix up the
> places that check the return code.
> 
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
> ---
>  fs/nfs/nfs4proc.c           | 12 +++++-------
>  fs/nfsd/nfs4callback.c      |  8 +++-----
>  include/linux/sunrpc/clnt.h |  4 ++--
>  net/sunrpc/clnt.c           |  7 +++----
>  4 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 405f17e6e0b45b26cebae06c5bbe932895af9a56..cda20bfeca56db1ef8c51e524d08908b93bfeba6 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -968,15 +968,13 @@ static int nfs41_sequence_process(struct rpc_task *task,
>  retry_new_seq:
>  	++slot->seq_nr;
>  retry_nowait:
> -	if (rpc_restart_call_prepare(task)) {
> -		nfs41_sequence_free_slot(res);
> -		task->tk_status = 0;
> -		ret = 0;
> -	}
> +	rpc_restart_call_prepare(task);
> +	nfs41_sequence_free_slot(res);
> +	task->tk_status = 0;
> +	ret = 0;
>  	goto out;
>  out_retry:
> -	if (!rpc_restart_call(task))
> -		goto out;
> +	rpc_restart_call(task);
>  	rpc_delay(task, NFS4_POLL_RETRY_MAX);
>  	return 0;
>  }
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index c083e539e898ba6593bc0d6185ccb8692aae6a95..b6d30111f7c85b0814ebd0821c4967574ca97e56 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1350,9 +1350,7 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
>  		goto need_restart;
>  	case -NFS4ERR_DELAY:
>  		cb->cb_seq_status = 1;
> -		if (!rpc_restart_call(task))
> -			goto out;
> -
> +		rpc_restart_call(task);
>  		rpc_delay(task, 2 * HZ);
>  		return false;
>  	case -NFS4ERR_BADSLOT:
> @@ -1374,8 +1372,8 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
>  out:
>  	return ret;
>  retry_nowait:
> -	if (rpc_restart_call_prepare(task))
> -		ret = false;
> +	rpc_restart_call_prepare(task);
> +	ret = false;
>  	goto out;
>  need_restart:
>  	if (!test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags)) {
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 5321585c778fcc1fef0e0420cb481786c02a7aac..e56f15c97fa24c735090c21c51ef312bfd877cfd 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -213,8 +213,8 @@ int		rpc_call_sync(struct rpc_clnt *clnt,
>  			      const struct rpc_message *msg, int flags);
>  struct rpc_task *rpc_call_null(struct rpc_clnt *clnt, struct rpc_cred *cred,
>  			       int flags);
> -int		rpc_restart_call_prepare(struct rpc_task *);
> -int		rpc_restart_call(struct rpc_task *);
> +void		rpc_restart_call_prepare(struct rpc_task *task);
> +void		rpc_restart_call(struct rpc_task *task);
>  void		rpc_setbufsize(struct rpc_clnt *, unsigned int, unsigned int);
>  struct net *	rpc_net_ns(struct rpc_clnt *);
>  size_t		rpc_max_payload(struct rpc_clnt *);
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 0090162ee8c350568c91f1bcd951675ac3ae141c..3d2989120599ccee32e8827b1790d4be7d7a565a 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -1670,20 +1670,19 @@ void rpc_force_rebind(struct rpc_clnt *clnt)
>  }
>  EXPORT_SYMBOL_GPL(rpc_force_rebind);
>  
> -static int
> +static void
>  __rpc_restart_call(struct rpc_task *task, void (*action)(struct rpc_task *))
>  {
>  	task->tk_status = 0;
>  	task->tk_rpc_status = 0;
>  	task->tk_action = action;
> -	return 1;
>  }
>  
>  /*
>   * Restart an (async) RPC call. Usually called from within the
>   * exit handler.
>   */
> -int
> +void
>  rpc_restart_call(struct rpc_task *task)
>  {
>  	return __rpc_restart_call(task, call_start);
> @@ -1694,7 +1693,7 @@ EXPORT_SYMBOL_GPL(rpc_restart_call);
>   * Restart an (async) RPC call from the call_prepare state.
>   * Usually called from within the exit handler.
>   */
> -int
> +void
>  rpc_restart_call_prepare(struct rpc_task *task)
>  {
>  	if (task->tk_ops->rpc_call_prepare != NULL)
> 
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250205-sunrpc-6-15-f19946041dfa
> 
> Best regards,

Reviewed-by: Chuck Lever <chuck.lever@...cle.com>


-- 
Chuck Lever