Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2010-06-15 at 12:08 -0400, Benny Halevy wrote:
> On Jun. 15, 2010, 11:50 -0400, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> > On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote:
> >> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> >>> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> >>> ---
> >>>  fs/nfs/nfs4proc.c       |   17 ++++++-----------
> >>>  include/linux/nfs_xdr.h |    6 ++++--
> >>>  2 files changed, 10 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >>> index ef613ea..e9913de 100644
> >>> --- a/fs/nfs/nfs4proc.c
> >>> +++ b/fs/nfs/nfs4proc.c
> >>> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
> >>>  	p->o_arg.server = server;
> >>>  	p->o_arg.bitmask = server->attr_bitmask;
> >>>  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
> >>> -	if (flags & O_EXCL) {
> >>> -		if (nfs4_has_persistent_session(server->nfs_client)) {
> >>> -			/* GUARDED */
> >>> -			p->o_arg.u.attrs = &p->attrs;
> >>> -			memcpy(&p->attrs, attrs, sizeof(p->attrs));
> >>
> >> What about this case?
> >> Aren't we losing the attrs?
> > 
> > I'm not sure what you mean. Are you asking what will happen if someone
> > sets O_EXCL without setting O_CREAT? We don't call encode_createmode at
> > all in that case. Otherwise, see below.
> > 
> 
> It looks like in the O_CREAT|O_EXCL case with persistent session
> the attrs passed in to nfs4_opendata_alloc are not copied to p->attrs
> and p->o_arg.u.attrs is not set.
> Then later on, in encode_createmode, in the respective path, we still use it:
> 			if (nfs4_has_persistent_session(clp)) {
> 				*p = cpu_to_be32(NFS4_CREATE_GUARDED);
> 				encode_attrs(xdr, arg->u.attrs, arg->server);
> 			} else {
> 
> Benny
> 
> > Cheers
> >   Trond
> > 
> >>
> >>> -		} else { /* EXCLUSIVE4_1 */
> >>> -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
> >>> -			s[0] = jiffies;
> >>> -			s[1] = current->pid;
> >>> -		}
> >>> -	} else if (flags & O_CREAT) {
> >>> +	if (flags & O_CREAT) {
> >>> +		u32 *s;
> >>> +
> >>>  		p->o_arg.u.attrs = &p->attrs;
> >>>  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Why isn't this sufficient?

> >>> +		s = (u32 *) p->o_arg.u.verifier.data;
> >>> +		s[0] = jiffies;
> >>> +		s[1] = current->pid;
> >>>  	}
> >>>  	p->c_arg.fh = &p->o_res.fh;
> >>>  	p->c_arg.stateid = &p->o_res.stateid;
> >>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> >>> index 51914d7..a319cb9 100644
> >>> --- a/include/linux/nfs_xdr.h
> >>> +++ b/include/linux/nfs_xdr.h
> >>> @@ -196,8 +196,10 @@ struct nfs_openargs {
> >>>  	__u64                   clientid;
> >>>  	__u64                   id;
> >>>  	union {
> >>> -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> >>> -		nfs4_verifier   verifier; /* EXCLUSIVE */
> >>> +		struct {
> >>> +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> >>> +			nfs4_verifier   verifier; /* EXCLUSIVE */
> >>> +		};
> >>>  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
> >>>  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
> >>>  	} u;
> >>
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux