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

[Jeff Layton <jlayton@kernel.org>]

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.
-- 
Jeff Layton <jlayton@kernel.org>