[pnfs] [PATCH 17/17] [SQUASHME] nfs41: optimize nfs41_setup_sequence for the minorversion 0 case

Ricardo Labiaga ricardo.labiaga at netapp.com
Wed Jun 4 17:08:15 EDT 2008


Agreed.

- ricardo


On Wed, 2008-06-04 at 11:05 +0300, Benny Halevy wrote:
> On Jun. 04, 2008, 2:33 +0300, Ricardo Labiaga <ricardo.labiaga at netapp.com> wrote:
> > Hi Benny,
> > 
> > I guess this is a matter of style.  Most of the minorversion decisions
> > were coded using a switch to specifically call out the places dependent
> > on minor version.  This would make it easy to deal with a new minor
> > version in the future.  Having said that, it's going to be so long
> > before that happens, that I really don't feel strongly either way.
> 
> Basically, I'd expect *and hope* that any minor version v > 1
> will continue to use sessions, and exchange_id in particular.
> For this we'd just need to change: BUG_ON(minorversion > v)
> 
> Benny
> 
> > 
> > - ricardo
> > 
> > 
> > On Tue, 2008-06-03 at 20:41 +0300, Benny Halevy wrote:
> >> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >> ---
> >>  fs/nfs/nfs4proc.c |   27 ++++++++++-----------------
> >>  1 files changed, 10 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index d4cf831..fed52ea 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -435,26 +435,19 @@ int nfs4_setup_sequence(struct nfs_client *clp,
> >>  
> >>  	dprintk("--> %s clp %p session %p cl_minorversion %d\n",
> >>  		__func__, clp, session, clp->cl_minorversion);
> >> -	switch (clp->cl_minorversion) {
> >> -	case 1:
> >> -		if (nfs41_test_session_expired(session)) {
> >> -			ret = nfs41_recover_session_sync(task->tk_client, clp,
> >> -							 session);
> >> -			if (ret)
> >> -				break;
> >> -		}
> >> -		ret = nfs41_setup_sequence(session, args,
> >> -				res, cache_reply, task);
> >> -		break;
> >> -	case 0:
> >> -		break;
> >> -	default:
> >> -		BUG();
> >> -	}
> >>  
> >> +	if (clp->cl_minorversion == 0)
> >> +		goto out;
> >> +	BUG_ON(clp->cl_minorversion != 1);
> >> +	if (nfs41_test_session_expired(session))
> >> +		ret = nfs41_recover_session_sync(task->tk_client, clp,
> >> +						 session);
> >> +	if (!ret)
> >> +		ret = nfs41_setup_sequence(session, args, res,
> >> +					    cache_reply, task);
> >>  	if (ret)
> >>  		memset(res, 0, sizeof(*res));
> >> -
> >> +out:
> >>  	dprintk("<-- %s status=%d\n", __func__, ret);
> >>  	return ret;
> >>  }
> 


More information about the pNFS mailing list