Re: [PATCH v3 4/6] NFSv4: Further cleanups to shutdown loops

[Trond Myklebust <trondmy@hammerspace.com>]

On Wed, 2025-03-26 at 14:21 -0400, Jeff Layton wrote:
> On Wed, 2025-03-26 at 18:11 +0000, Trond Myklebust wrote:
> > On Wed, 2025-03-26 at 06:13 -0400, Jeff Layton wrote:
> > > On Tue, 2025-03-25 at 18:35 -0400, trondmy@kernel.org wrote:
> > > > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > > 
> > > > Replace the tests for the RPC client being shut down with tests
> > > > for
> > > > whether the nfs_client is in an error state.
> > > > 
> > > > Signed-off-by: Trond Myklebust
> > > > <trond.myklebust@hammerspace.com>
> > > > ---
> > > >  fs/nfs/nfs4proc.c  | 2 +-
> > > >  fs/nfs/nfs4state.c | 2 +-
> > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > index 889511650ceb..50be54e0f578 100644
> > > > --- a/fs/nfs/nfs4proc.c
> > > > +++ b/fs/nfs/nfs4proc.c
> > > > @@ -9580,7 +9580,7 @@ static void
> > > > nfs41_sequence_call_done(struct
> > > > rpc_task *task, void *data)
> > > >  		return;
> > > >  
> > > >  	trace_nfs4_sequence(clp, task->tk_status);
> > > > -	if (task->tk_status < 0 && !task->tk_client-
> > > > >cl_shutdown)
> > > > {
> > > > +	if (task->tk_status < 0 && clp->cl_cons_state >= 0) {
> > > >  		dprintk("%s ERROR %d\n", __func__, task-
> > > > > tk_status);
> > > >  		if (refcount_read(&clp->cl_count) == 1)
> > > >  			return;
> > > > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > > > index 542cdf71229f..f1f7eaa97973 100644
> > > > --- a/fs/nfs/nfs4state.c
> > > > +++ b/fs/nfs/nfs4state.c
> > > > @@ -1198,7 +1198,7 @@ void nfs4_schedule_state_manager(struct
> > > > nfs_client *clp)
> > > >  	struct rpc_clnt *clnt = clp->cl_rpcclient;
> > > >  	bool swapon = false;
> > > >  
> > > > -	if (clnt->cl_shutdown)
> > > > +	if (clp->cl_cons_state < 0)
> > > >  		return;
> > > >  
> > > >  	set_bit(NFS4CLNT_RUN_MANAGER, &clp->cl_state);
> > > 
> > > One more thing:
> > > 
> > > Do we need cl_shutdown at all? If we can replace these checks
> > > here
> > > with
> > > a check for cl_cons_state < 0, why not do the same in
> > > call_start()?
> > 
> > The struct nfs_client is a NFS level object. It can't be moved to
> > the
> > RPC layer.
> > 
> 
> But...cl_shutdown is an rpc_clnt field.

Right, but cl_cons_state is a field in struct nfs_client, hence that
check cannot be moved into call_start()

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com