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
next prev parent 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