Skip to content

Commit ca0daa2

Browse files
trondmypdtrondmypd
committed
NFS: Cache aggressively when file is open for writing
Unless the user is using file locking, we must assume close-to-open cache consistency when the file is open for writing. Adjust the caching algorithm so that it does not clear the cache on out-of-order writes and/or attribute revalidations. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
1 parent 57b6918 commit ca0daa2

File tree

2 files changed

+46
-29
lines changed

2 files changed

+46
-29
lines changed

‎fs/nfs/file.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -779,11 +779,6 @@ do_unlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
779779
return status;
780780
}
781781

782-
static int
783-
is_time_granular(struct timespec *ts) {
784-
return ((ts->tv_sec == 0) && (ts->tv_nsec <= 1000));
785-
}
786-
787782
static int
788783
do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
789784
{
@@ -817,12 +812,8 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
817812
* This makes locking act as a cache coherency point.
818813
*/
819814
nfs_sync_mapping(filp->f_mapping);
820-
if (!NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) {
821-
if (is_time_granular(&NFS_SERVER(inode)->time_delta))
822-
__nfs_revalidate_inode(NFS_SERVER(inode), inode);
823-
else
824-
nfs_zap_caches(inode);
825-
}
815+
if (!NFS_PROTO(inode)->have_delegation(inode, FMODE_READ))
816+
nfs_zap_mapping(inode, filp->f_mapping);
826817
out:
827818
return status;
828819
}

‎fs/nfs/inode.c

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,10 @@ void nfs_inode_attach_open_context(struct nfs_open_context *ctx)
878878
struct nfs_inode *nfsi = NFS_I(inode);
879879

880880
spin_lock(&inode->i_lock);
881-
list_add(&ctx->list, &nfsi->open_files);
881+
if (ctx->mode & FMODE_WRITE)
882+
list_add(&ctx->list, &nfsi->open_files);
883+
else
884+
list_add_tail(&ctx->list, &nfsi->open_files);
882885
spin_unlock(&inode->i_lock);
883886
}
884887
EXPORT_SYMBOL_GPL(nfs_inode_attach_open_context);
@@ -1215,6 +1218,25 @@ int nfs_revalidate_mapping_protected(struct inode *inode, struct address_space *
12151218
return __nfs_revalidate_mapping(inode, mapping, true);
12161219
}
12171220

1221+
static bool nfs_file_has_writers(struct nfs_inode *nfsi)
1222+
{
1223+
struct inode *inode = &nfsi->vfs_inode;
1224+
1225+
assert_spin_locked(&inode->i_lock);
1226+
1227+
if (!S_ISREG(inode->i_mode))
1228+
return false;
1229+
if (list_empty(&nfsi->open_files))
1230+
return false;
1231+
/* Note: This relies on nfsi->open_files being ordered with writers
1232+
* being placed at the head of the list.
1233+
* See nfs_inode_attach_open_context()
1234+
*/
1235+
return (list_first_entry(&nfsi->open_files,
1236+
struct nfs_open_context,
1237+
list)->mode & FMODE_WRITE) == FMODE_WRITE;
1238+
}
1239+
12181240
static unsigned long nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr)
12191241
{
12201242
struct nfs_inode *nfsi = NFS_I(inode);
@@ -1279,22 +1301,24 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
12791301
if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
12801302
return -EIO;
12811303

1282-
if ((fattr->valid & NFS_ATTR_FATTR_CHANGE) != 0 &&
1283-
inode->i_version != fattr->change_attr)
1284-
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
1304+
if (!nfs_file_has_writers(nfsi)) {
1305+
/* Verify a few of the more important attributes */
1306+
if ((fattr->valid & NFS_ATTR_FATTR_CHANGE) != 0 && inode->i_version != fattr->change_attr)
1307+
invalid |= NFS_INO_INVALID_ATTR | NFS_INO_REVAL_PAGECACHE;
12851308

1286-
/* Verify a few of the more important attributes */
1287-
if ((fattr->valid & NFS_ATTR_FATTR_MTIME) && !timespec_equal(&inode->i_mtime, &fattr->mtime))
1288-
invalid |= NFS_INO_INVALID_ATTR;
1309+
if ((fattr->valid & NFS_ATTR_FATTR_MTIME) && !timespec_equal(&inode->i_mtime, &fattr->mtime))
1310+
invalid |= NFS_INO_INVALID_ATTR;
12891311

1290-
if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
1291-
cur_size = i_size_read(inode);
1292-
new_isize = nfs_size_to_loff_t(fattr->size);
1293-
if (cur_size != new_isize)
1294-
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
1312+
if ((fattr->valid & NFS_ATTR_FATTR_CTIME) && !timespec_equal(&inode->i_ctime, &fattr->ctime))
1313+
invalid |= NFS_INO_INVALID_ATTR;
1314+
1315+
if (fattr->valid & NFS_ATTR_FATTR_SIZE) {
1316+
cur_size = i_size_read(inode);
1317+
new_isize = nfs_size_to_loff_t(fattr->size);
1318+
if (cur_size != new_isize)
1319+
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
1320+
}
12951321
}
1296-
if (nfsi->nrequests != 0)
1297-
invalid &= ~NFS_INO_REVAL_PAGECACHE;
12981322

12991323
/* Have any file permissions changed? */
13001324
if ((fattr->valid & NFS_ATTR_FATTR_MODE) && (inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO))
@@ -1526,7 +1550,7 @@ EXPORT_SYMBOL_GPL(nfs_refresh_inode);
15261550

15271551
static int nfs_post_op_update_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
15281552
{
1529-
unsigned long invalid = NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE;
1553+
unsigned long invalid = NFS_INO_INVALID_ATTR;
15301554

15311555
/*
15321556
* Don't revalidate the pagecache if we hold a delegation, but do
@@ -1675,6 +1699,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
16751699
unsigned long invalid = 0;
16761700
unsigned long now = jiffies;
16771701
unsigned long save_cache_validity;
1702+
bool have_writers = nfs_file_has_writers(nfsi);
16781703
bool cache_revalidated = true;
16791704

16801705
dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n",
@@ -1730,7 +1755,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
17301755
dprintk("NFS: change_attr change on server for file %s/%ld\n",
17311756
inode->i_sb->s_id, inode->i_ino);
17321757
/* Could it be a race with writeback? */
1733-
if (nfsi->nrequests == 0) {
1758+
if (!have_writers) {
17341759
invalid |= NFS_INO_INVALID_ATTR
17351760
| NFS_INO_INVALID_DATA
17361761
| NFS_INO_INVALID_ACCESS
@@ -1770,9 +1795,10 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
17701795
if (new_isize != cur_isize) {
17711796
/* Do we perhaps have any outstanding writes, or has
17721797
* the file grown beyond our last write? */
1773-
if ((nfsi->nrequests == 0) || new_isize > cur_isize) {
1798+
if (nfsi->nrequests == 0 || new_isize > cur_isize) {
17741799
i_size_write(inode, new_isize);
1775-
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
1800+
if (!have_writers)
1801+
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
17761802
}
17771803
dprintk("NFS: isize change on server for file %s/%ld "
17781804
"(%Ld to %Ld)\n",

0 commit comments

Comments
 (0)