nfsd4: factor ctime into change attribute
authorJ. Bruce Fields <bfields@redhat.com>
Thu, 11 May 2017 18:45:06 +0000 (14:45 -0400)
committerJ. Bruce Fields <bfields@redhat.com>
Wed, 12 Jul 2017 19:55:00 +0000 (15:55 -0400)
Factoring ctime into the nfsv4 change attribute gives us better
properties than just i_version alone.

Eventually we'll likely also expose this (as opposed to raw i_version)
to userspace, at which point we'll want to move it to a common helper,
called from either userspace or individual filesystems.  For now, nfsd
is the only user.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
fs/nfsd/nfs3xdr.c
fs/nfsd/nfs4xdr.c
fs/nfsd/nfsfh.h

index b8838d3023ff2e61fb8e04e02736780ab38b00cd..bf444b664011a891992b04435b739e30fbc17b0e 100644 (file)
@@ -260,7 +260,7 @@ void fill_post_wcc(struct svc_fh *fhp)
                printk("nfsd: inode locked twice during operation.\n");
 
        err = fh_getattr(fhp, &fhp->fh_post_attr);
-       fhp->fh_post_change = d_inode(fhp->fh_dentry)->i_version;
+       fhp->fh_post_change = nfsd4_change_attribute(d_inode(fhp->fh_dentry));
        if (err) {
                fhp->fh_post_saved = false;
                /* Grab the ctime anyway - set_change_info might use it */
index 54e212e3541eb1ee26e968667a2859833ac1e177..20fbcab977531bee75501a9403f4c8f60ee404d4 100644 (file)
@@ -1973,7 +1973,7 @@ static __be32 *encode_change(__be32 *p, struct kstat *stat, struct inode *inode,
                *p++ = cpu_to_be32(convert_to_wallclock(exp->cd->flush_time));
                *p++ = 0;
        } else if (IS_I_VERSION(inode)) {
-               p = xdr_encode_hyper(p, inode->i_version);
+               p = xdr_encode_hyper(p, nfsd4_change_attribute(inode));
        } else {
                *p++ = cpu_to_be32(stat->ctime.tv_sec);
                *p++ = cpu_to_be32(stat->ctime.tv_nsec);
index f84fe6bf9aee144cd7227711540f0027e24ef6c2..e47cf6c2ac28b6ad1aac5797a9ecb93806e2f951 100644 (file)
@@ -240,6 +240,28 @@ fh_clear_wcc(struct svc_fh *fhp)
        fhp->fh_pre_saved = false;
 }
 
+/*
+ * We could use i_version alone as the change attribute.  However,
+ * i_version can go backwards after a reboot.  On its own that doesn't
+ * necessarily cause a problem, but if i_version goes backwards and then
+ * is incremented again it could reuse a value that was previously used
+ * before boot, and a client who queried the two values might
+ * incorrectly assume nothing changed.
+ *
+ * By using both ctime and the i_version counter we guarantee that as
+ * long as time doesn't go backwards we never reuse an old value.
+ */
+static inline u64 nfsd4_change_attribute(struct inode *inode)
+{
+       u64 chattr;
+
+       chattr =  inode->i_ctime.tv_sec;
+       chattr <<= 30;
+       chattr += inode->i_ctime.tv_nsec;
+       chattr += inode->i_version;
+       return chattr;
+}
+
 /*
  * Fill in the pre_op attr for the wcc data
  */
@@ -253,7 +275,7 @@ fill_pre_wcc(struct svc_fh *fhp)
                fhp->fh_pre_mtime = inode->i_mtime;
                fhp->fh_pre_ctime = inode->i_ctime;
                fhp->fh_pre_size  = inode->i_size;
-               fhp->fh_pre_change = inode->i_version;
+               fhp->fh_pre_change = nfsd4_change_attribute(inode);
                fhp->fh_pre_saved = true;
        }
 }