NFSv4.2: fix setattr caching of TIME_[MODIFY|ACCESS]_SET when timestamps are delegated

nfs_setattr will flush all pending writes before updating a file time
attributes. However when the client holds delegated timestamps, it can
update its timestamps locally as it is the authority for the file
times attributes. The client will later set the file attributes by
adding a setattr to the delegreturn compound updating the server time
attributes.

Fix nfs_setattr to avoid flushing pending writes when the file time
attributes are delegated and the mtime/atime are set to a fixed
timestamp (ATTR_[MODIFY|ACCESS]_SET. Also, when sending the setattr
procedure over the wire, we need to clear the correct attribute bits
from the bitmask.

I was able to measure a noticable speedup when measuring untar performance.
Test: $ time tar xzf ~/dir.tgz
Baseline: 1m13.072s
Patched: 0m49.038s

Which is more than 30% latency improvement.

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 119e447758b994b34e55e7b28fd4f34fa089e2e1..6472b95bfd883003f6aab3f9e58922cce4e7e166 100644 (file)
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -633,6 +633,34 @@ nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr *fattr)
        }
 }
 
+static void nfs_set_timestamps_to_ts(struct inode *inode, struct iattr *attr)
+{
+       unsigned int cache_flags = 0;
+
+       if (attr->ia_valid & ATTR_MTIME_SET) {
+               struct timespec64 ctime = inode_get_ctime(inode);
+               struct timespec64 mtime = inode_get_mtime(inode);
+               struct timespec64 now;
+               int updated = 0;
+
+               now = inode_set_ctime_current(inode);
+               if (!timespec64_equal(&now, &ctime))
+                       updated |= S_CTIME;
+
+               inode_set_mtime_to_ts(inode, attr->ia_mtime);
+               if (!timespec64_equal(&now, &mtime))
+                       updated |= S_MTIME;
+
+               inode_maybe_inc_iversion(inode, updated);
+               cache_flags |= NFS_INO_INVALID_CTIME | NFS_INO_INVALID_MTIME;
+       }
+       if (attr->ia_valid & ATTR_ATIME_SET) {
+               inode_set_atime_to_ts(inode, attr->ia_atime);
+               cache_flags |= NFS_INO_INVALID_ATIME;
+       }
+       NFS_I(inode)->cache_validity &= ~cache_flags;
+}
+
 static void nfs_update_timestamps(struct inode *inode, unsigned int ia_valid)
 {
        enum file_time_flags time_flags = 0;
@@ -701,14 +729,27 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 
        if (nfs_have_delegated_mtime(inode) && attr->ia_valid & ATTR_MTIME) {
                spin_lock(&inode->i_lock);
-               nfs_update_timestamps(inode, attr->ia_valid);
+               if (attr->ia_valid & ATTR_MTIME_SET) {
+                       nfs_set_timestamps_to_ts(inode, attr);
+                       attr->ia_valid &= ~(ATTR_MTIME|ATTR_MTIME_SET|
+                                               ATTR_ATIME|ATTR_ATIME_SET);
+               } else {
+                       nfs_update_timestamps(inode, attr->ia_valid);
+                       attr->ia_valid &= ~(ATTR_MTIME|ATTR_ATIME);
+               }
                spin_unlock(&inode->i_lock);
-               attr->ia_valid &= ~(ATTR_MTIME | ATTR_ATIME);
        } else if (nfs_have_delegated_atime(inode) &&
                   attr->ia_valid & ATTR_ATIME &&
                   !(attr->ia_valid & ATTR_MTIME)) {
-               nfs_update_delegated_atime(inode);
-               attr->ia_valid &= ~ATTR_ATIME;
+               if (attr->ia_valid & ATTR_ATIME_SET) {
+                       spin_lock(&inode->i_lock);
+                       nfs_set_timestamps_to_ts(inode, attr);
+                       spin_unlock(&inode->i_lock);
+                       attr->ia_valid &= ~(ATTR_ATIME|ATTR_ATIME_SET);
+               } else {
+                       nfs_update_delegated_atime(inode);
+                       attr->ia_valid &= ~ATTR_ATIME;
+               }
        }
 
        /* Optimization: if the end result is no change, don't RPC */

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a7c4194ed2a0bb45444790393a5b88e82592bb10..87f9f6fb021439bf97977e5648524d76c2277378 100644 (file)
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -325,14 +325,14 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
 
        if (nfs_have_delegated_mtime(inode)) {
                if (!(cache_validity & NFS_INO_INVALID_ATIME))
-                       dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
+                       dst[1] &= ~(FATTR4_WORD1_TIME_ACCESS|FATTR4_WORD1_TIME_ACCESS_SET);
                if (!(cache_validity & NFS_INO_INVALID_MTIME))
-                       dst[1] &= ~FATTR4_WORD1_TIME_MODIFY;
+                       dst[1] &= ~(FATTR4_WORD1_TIME_MODIFY|FATTR4_WORD1_TIME_MODIFY_SET);
                if (!(cache_validity & NFS_INO_INVALID_CTIME))
-                       dst[1] &= ~FATTR4_WORD1_TIME_METADATA;
+                       dst[1] &= ~(FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY_SET);
        } else if (nfs_have_delegated_atime(inode)) {
                if (!(cache_validity & NFS_INO_INVALID_ATIME))
-                       dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
+                       dst[1] &= ~(FATTR4_WORD1_TIME_ACCESS|FATTR4_WORD1_TIME_ACCESS_SET);
        }
 }