Re: [PATCH 1/5] NFSv4: Fix an atomicity problem in CLOSE

[Jeff Layton <jeff.layton@primarydata.com>]

On Sat, 24 Jan 2015 06:46:28 -0500
Jeff Layton <jeff.layton@primarydata.com> wrote:

> On Sat, 24 Jan 2015 00:39:24 -0500
> Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> 
> > If we are to remove the serialisation of OPEN/CLOSE, then we need to
> > ensure that the stateid sent as part of a CLOSE operation does not
> > change after we test the state in nfs4_close_prepare.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> > ---
> >  fs/nfs/nfs4proc.c       | 7 ++++++-
> >  fs/nfs/nfs4xdr.c        | 4 ++--
> >  include/linux/nfs_xdr.h | 2 +-
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index c347705b0161..4863dec10865 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -2587,6 +2587,11 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
> >  		case -NFS4ERR_OLD_STATEID:
> >  		case -NFS4ERR_BAD_STATEID:
> >  		case -NFS4ERR_EXPIRED:
> > +			if (!nfs4_stateid_match(&calldata->arg.stateid,
> > +						&state->stateid)) {
> > +				rpc_restart_call_prepare(task);
> > +				goto out_release;
> > +			}
> 
> Do we need a similar check in the open codepath -- possibly in
> nfs4_open_done? AFAICT, currently if the OPEN ends up "losing" the race
> here, then we'll fall into full-on stateid recovery, which is almost
> certainly not what we want.
> 

(facepalm)

Oh nm...stupid q. The OPEN can't lose the race since you won't
necessarily have a stateid at the start of the call, and you're
upgrading anyway.

I think you're correct that CLOSE is the only place you have to worry
about it.

-- 
Jeff Layton <jeff.layton@primarydata.com>