Skip to content

Commit aba41e9

Browse files
sagigrimbergAnna Schumaker
authored andcommitted
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 <[email protected]> Signed-off-by: Anna Schumaker <[email protected]>
1 parent d2e1d78 commit aba41e9

File tree

2 files changed

+49
-8
lines changed

2 files changed

+49
-8
lines changed

fs/nfs/inode.c

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,34 @@ nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr *fattr)
633633
}
634634
}
635635

636+
static void nfs_set_timestamps_to_ts(struct inode *inode, struct iattr *attr)
637+
{
638+
unsigned int cache_flags = 0;
639+
640+
if (attr->ia_valid & ATTR_MTIME_SET) {
641+
struct timespec64 ctime = inode_get_ctime(inode);
642+
struct timespec64 mtime = inode_get_mtime(inode);
643+
struct timespec64 now;
644+
int updated = 0;
645+
646+
now = inode_set_ctime_current(inode);
647+
if (!timespec64_equal(&now, &ctime))
648+
updated |= S_CTIME;
649+
650+
inode_set_mtime_to_ts(inode, attr->ia_mtime);
651+
if (!timespec64_equal(&now, &mtime))
652+
updated |= S_MTIME;
653+
654+
inode_maybe_inc_iversion(inode, updated);
655+
cache_flags |= NFS_INO_INVALID_CTIME | NFS_INO_INVALID_MTIME;
656+
}
657+
if (attr->ia_valid & ATTR_ATIME_SET) {
658+
inode_set_atime_to_ts(inode, attr->ia_atime);
659+
cache_flags |= NFS_INO_INVALID_ATIME;
660+
}
661+
NFS_I(inode)->cache_validity &= ~cache_flags;
662+
}
663+
636664
static void nfs_update_timestamps(struct inode *inode, unsigned int ia_valid)
637665
{
638666
enum file_time_flags time_flags = 0;
@@ -701,14 +729,27 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
701729

702730
if (nfs_have_delegated_mtime(inode) && attr->ia_valid & ATTR_MTIME) {
703731
spin_lock(&inode->i_lock);
704-
nfs_update_timestamps(inode, attr->ia_valid);
732+
if (attr->ia_valid & ATTR_MTIME_SET) {
733+
nfs_set_timestamps_to_ts(inode, attr);
734+
attr->ia_valid &= ~(ATTR_MTIME|ATTR_MTIME_SET|
735+
ATTR_ATIME|ATTR_ATIME_SET);
736+
} else {
737+
nfs_update_timestamps(inode, attr->ia_valid);
738+
attr->ia_valid &= ~(ATTR_MTIME|ATTR_ATIME);
739+
}
705740
spin_unlock(&inode->i_lock);
706-
attr->ia_valid &= ~(ATTR_MTIME | ATTR_ATIME);
707741
} else if (nfs_have_delegated_atime(inode) &&
708742
attr->ia_valid & ATTR_ATIME &&
709743
!(attr->ia_valid & ATTR_MTIME)) {
710-
nfs_update_delegated_atime(inode);
711-
attr->ia_valid &= ~ATTR_ATIME;
744+
if (attr->ia_valid & ATTR_ATIME_SET) {
745+
spin_lock(&inode->i_lock);
746+
nfs_set_timestamps_to_ts(inode, attr);
747+
spin_unlock(&inode->i_lock);
748+
attr->ia_valid &= ~(ATTR_ATIME|ATTR_ATIME_SET);
749+
} else {
750+
nfs_update_delegated_atime(inode);
751+
attr->ia_valid &= ~ATTR_ATIME;
752+
}
712753
}
713754

714755
/* Optimization: if the end result is no change, don't RPC */

fs/nfs/nfs4proc.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,14 @@ static void nfs4_bitmap_copy_adjust(__u32 *dst, const __u32 *src,
325325

326326
if (nfs_have_delegated_mtime(inode)) {
327327
if (!(cache_validity & NFS_INO_INVALID_ATIME))
328-
dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
328+
dst[1] &= ~(FATTR4_WORD1_TIME_ACCESS|FATTR4_WORD1_TIME_ACCESS_SET);
329329
if (!(cache_validity & NFS_INO_INVALID_MTIME))
330-
dst[1] &= ~FATTR4_WORD1_TIME_MODIFY;
330+
dst[1] &= ~(FATTR4_WORD1_TIME_MODIFY|FATTR4_WORD1_TIME_MODIFY_SET);
331331
if (!(cache_validity & NFS_INO_INVALID_CTIME))
332-
dst[1] &= ~FATTR4_WORD1_TIME_METADATA;
332+
dst[1] &= ~(FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY_SET);
333333
} else if (nfs_have_delegated_atime(inode)) {
334334
if (!(cache_validity & NFS_INO_INVALID_ATIME))
335-
dst[1] &= ~FATTR4_WORD1_TIME_ACCESS;
335+
dst[1] &= ~(FATTR4_WORD1_TIME_ACCESS|FATTR4_WORD1_TIME_ACCESS_SET);
336336
}
337337
}
338338

0 commit comments

Comments
 (0)