Linux NFS development
 help / color / mirror / Atom feed
From: Stuart Anderson <anderson@ligo.caltech.edu>
To: Trond Myklebust <trond.myklebust@fys.uio.no>, jlayton@redhat.com
Cc: nfs@lists.sourceforge.net
Subject: Re: NFSv4 uninitialized mtime
Date: Wed, 27 Jun 2007 17:49:04 -0700	[thread overview]
Message-ID: <20070628004904.GK9806@ligo.caltech.edu> (raw)
In-Reply-To: <1182987805.5311.77.camel@heimdal.trondhjem.org>

Unfortunately this patch did not have any observable affects.

On Wed, Jun 27, 2007 at 07:43:25PM -0400, Trond Myklebust wrote:
> On Wed, 2007-06-27 at 16:31 -0700, Stuart Anderson wrote:
> > The following simple program creates files with un-initialized mtime values
> > on a Linux NFSv4 client mounting from a Solaris NFSv4 server--at least the
> > mtimes are wildly different each time the program is run. The problem is
> > reproducible but does not happen under any of the following circumstances:
> > 
> > 1) Drop O_EXCL from open() call.
> > 2) NFS mount using v3.
> > 3) Switch NFS v4 client from Linux to Solaris.
> > 
> > This is on an FC4 machine with the 2.6.20.14 kernel plus Trond's recent
> > revalidate-the-fsid-on-the-current-dir-not-the-root-dir patch
> > and nfs-utils 1.0.9-16 backported from CentOS 5.
> > 
> > Thanks.
> 
> Doesn't Jeff's patch fix it?
> 
> Trond
> 
> 

> From: Jeff Layton <jlayton@redhat.com>
> Date: Tue, 5 Jun 2007 14:49:03 -0400
> NFS4: on a O_EXCL OPEN make sure SETATTR sets the fields holding the
> 	verifier
> Subject: No Subject
> 
> The Linux NFS4 client simply skips over the bitmask in an O_EXCL open
> call and so it doesn't bother to reset any fields that may be holding
> the verifier. This patch has us save the first two words of the bitmask
> (which is all the current client has #defines for). The client then
> later checks this bitmask and turns on the appropriate flags in the
> sattr->ia_verify field for the following SETATTR call.
> 
> This patch only currently checks to see if the server used the atime
> and mtime slots for the verifier (which is what the Linux server uses
> for this). I'm not sure of what other fields the server could
> reasonably use, but adding checks for others should be trivial.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfs/nfs4proc.c       |   20 ++++++++++++++++++++
>  fs/nfs/nfs4xdr.c        |    9 +++++++--
>  include/linux/nfs4.h    |    1 +
>  include/linux/nfs_xdr.h |    1 +
>  4 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 3cc7544..fee2d14 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -943,6 +943,22 @@ static struct nfs4_state *nfs4_open_delegated(struct inode *inode, int flags, st
>  }
>  
>  /*
> + * on an EXCLUSIVE create, the server should send back a bitmask with FATTR4-*
> + * fields corresponding to attributes that were used to store the verifier.
> + * Make sure we clobber those fields in the later setattr call
> + */
> +static inline void nfs4_exclusive_attrset(struct nfs4_opendata *opendata, struct iattr *sattr)
> +{
> +	if ((opendata->o_res.attrset[1] & FATTR4_WORD1_TIME_ACCESS) &&
> +	    !(sattr->ia_valid & ATTR_ATIME_SET))
> +		sattr->ia_valid |= ATTR_ATIME;
> +
> +	if ((opendata->o_res.attrset[1] & FATTR4_WORD1_TIME_MODIFY) &&
> +	    !(sattr->ia_valid & ATTR_MTIME_SET))
> +		sattr->ia_valid |= ATTR_MTIME;
> +}
> +
> +/*
>   * Returns a referenced nfs4_state
>   */
>  static int _nfs4_do_open(struct inode *dir, struct path *path, int flags, struct iattr *sattr, struct rpc_cred *cred, struct nfs4_state **res)
> @@ -973,6 +989,9 @@ static int _nfs4_do_open(struct inode *dir, struct path *path, int flags, struct
>  	if (status != 0)
>  		goto err_opendata_free;
>  
> +	if (opendata->o_arg.open_flags & O_EXCL)
> +		nfs4_exclusive_attrset(opendata, sattr);
> +
>  	status = -ENOMEM;
>  	state = nfs4_opendata_to_nfs4_state(opendata);
>  	if (state == NULL)
> @@ -1784,6 +1803,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
>  		status = nfs4_do_setattr(state->inode, &fattr, sattr, state);
>  		if (status == 0)
>  			nfs_setattr_update_inode(state->inode, sattr);
> +		nfs_post_op_update_inode(state->inode, &fattr);
>  	}
>  	if (status == 0 && (nd->flags & LOOKUP_OPEN) != 0)
>  		status = nfs4_intent_set_file(nd, &path, state);
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 8003c91..5efd314 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -3269,7 +3269,7 @@ static int decode_delegation(struct xdr_stream *xdr, struct nfs_openres *res)
>  static int decode_open(struct xdr_stream *xdr, struct nfs_openres *res)
>  {
>          __be32 *p;
> -        uint32_t bmlen;
> +        uint32_t savewords, bmlen, i;
>          int status;
>  
>          status = decode_op_hdr(xdr, OP_OPEN);
> @@ -3287,7 +3287,12 @@ static int decode_open(struct xdr_stream *xdr, struct nfs_openres *res)
>                  goto xdr_error;
>  
>          READ_BUF(bmlen << 2);
> -        p += bmlen;
> +	savewords = min_t(uint32_t, bmlen, NFS4_BITMAP_SIZE);
> +	for (i = 0; i < savewords; ++i)
> +		READ32(res->attrset[i]);
> +
> +	p += (bmlen - savewords);
> +
>  	return decode_delegation(xdr, res);
>  xdr_error:
>  	dprintk("%s: Bitmap too large! Length = %u\n", __FUNCTION__, bmlen);
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 7e7f33a..8726491 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -15,6 +15,7 @@
>  
>  #include <linux/types.h>
>  
> +#define NFS4_BITMAP_SIZE	2
>  #define NFS4_VERIFIER_SIZE	8
>  #define NFS4_STATEID_SIZE	16
>  #define NFS4_FHSIZE		128
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 10c26ed..f7100df 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -144,6 +144,7 @@ struct nfs_openres {
>  	nfs4_stateid		delegation;
>  	__u32			do_recall;
>  	__u64			maxsize;
> +	__u32			attrset[NFS4_BITMAP_SIZE];
>  };
>  
>  /*


-- 
Stuart Anderson  anderson@ligo.caltech.edu
http://www.ligo.caltech.edu/~anderson

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

  reply	other threads:[~2007-06-28  0:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-27 23:31 NFSv4 uninitialized mtime Stuart Anderson
2007-06-27 23:43 ` Trond Myklebust
2007-06-28  0:49   ` Stuart Anderson [this message]
2007-06-28  0:59     ` Stuart Anderson
2007-06-28  1:15       ` Jeff Layton
2007-06-28  2:53         ` Stuart Anderson
2007-06-28  3:09           ` Spencer Shepler
2007-06-28  3:23             ` Stuart Anderson
2007-06-28  3:30               ` Spencer Shepler
2007-06-28  3:44                 ` Stuart Anderson
2007-06-28  3:59                   ` Spencer Shepler
2007-06-28 13:32                     ` J. Bruce Fields
2007-06-28 10:41           ` Jeff Layton
2007-06-28 12:01             ` Jeff Layton
2007-06-28 13:19               ` Trond Myklebust
2007-06-28 13:29                 ` Jeff Layton
     [not found] <nfs-valinux.20070628064127.a769bc53.jlayton@redhat.com>
     [not found] ` <46857F06.102@ligo.caltech.edu>
     [not found]   ` <20070629181234.b70f6f7f.jlayton@redhat.com>
2007-06-29 22:31     ` Erik A. Espinoza
2007-06-30  2:35       ` Jeff Layton
2007-06-30  3:09         ` Erik A. Espinoza
2007-06-30 11:19           ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070628004904.GK9806@ligo.caltech.edu \
    --to=anderson@ligo.caltech.edu \
    --cc=jlayton@redhat.com \
    --cc=nfs@lists.sourceforge.net \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox