2016-06-26 00:26:38

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 1/5] NFS: Do not aggressively cache file attributes in the case of O_DIRECT

A file that is open for O_DIRECT is not guaranteed to obey close-to-open
cache consistency semantics, so let's not cache attributes aggressively.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/inode.c | 9 +++++++--
fs/nfs/internal.h | 5 +++++
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index b3404b676946..b729f7b972d3 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1207,6 +1207,11 @@ static bool nfs_file_has_writers(struct nfs_inode *nfsi)
list)->mode & FMODE_WRITE) == FMODE_WRITE;
}

+static bool nfs_file_has_buffered_writers(struct nfs_inode *nfsi)
+{
+ return nfs_file_has_writers(nfsi) && nfs_file_io_is_buffered(nfsi);
+}
+
static unsigned long nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr)
{
struct nfs_inode *nfsi = NFS_I(inode);
@@ -1271,7 +1276,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
if ((fattr->valid & NFS_ATTR_FATTR_TYPE) && (inode->i_mode & S_IFMT) != (fattr->mode & S_IFMT))
return -EIO;

- if (!nfs_file_has_writers(nfsi)) {
+ if (!nfs_file_has_buffered_writers(nfsi)) {
/* Verify a few of the more important attributes */
if ((fattr->valid & NFS_ATTR_FATTR_CHANGE) != 0 && inode->i_version != fattr->change_attr)
invalid |= NFS_INO_INVALID_ATTR | NFS_INO_REVAL_PAGECACHE;
@@ -1669,7 +1674,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
unsigned long invalid = 0;
unsigned long now = jiffies;
unsigned long save_cache_validity;
- bool have_writers = nfs_file_has_writers(nfsi);
+ bool have_writers = nfs_file_has_buffered_writers(nfsi);
bool cache_revalidated = true;

dfprintk(VFS, "NFS: %s(%s/%lu fh_crc=0x%08x ct=%d info=0x%x)\n",
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 159b64ede82a..01dccf18da0a 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -419,6 +419,11 @@ extern void nfs_end_io_write(struct inode *inode);
extern void nfs_start_io_direct(struct inode *inode);
extern void nfs_end_io_direct(struct inode *inode);

+static inline bool nfs_file_io_is_buffered(struct nfs_inode *nfsi)
+{
+ return test_bit(NFS_INO_ODIRECT, &nfsi->flags) == 0;
+}
+
/* namespace.c */
#define NFS_PATH_CANONICAL 1
extern char *nfs_path(char **p, struct dentry *dentry,
--
2.7.4



2016-06-26 00:26:38

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 2/5] NFS: Fix a race in nfs42_proc_deallocate()

When punching holes in a file, we want to ensure the operation is
serialised w.r.t. other writes, meaning that we want to call
nfs_sync_inode() while holding the inode lock.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs42proc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index aa03ed09ba06..0f9f536e647b 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -113,15 +113,17 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
if (!nfs_server_capable(inode, NFS_CAP_DEALLOCATE))
return -EOPNOTSUPP;

- nfs_wb_all(inode);
inode_lock(inode);
+ err = nfs_sync_inode(inode);
+ if (err)
+ goto out_unlock;

err = nfs42_proc_fallocate(&msg, filep, offset, len);
if (err == 0)
truncate_pagecache_range(inode, offset, (offset + len) -1);
if (err == -EOPNOTSUPP)
NFS_SERVER(inode)->caps &= ~NFS_CAP_DEALLOCATE;
-
+out_unlock:
inode_unlock(inode);
return err;
}
--
2.7.4


2016-06-26 00:26:41

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 3/5] NFSv4.2: Fix writeback races in nfs4_copy_file_range

We need to ensure that any writes to the destination file are serialised
with the copy, meaning that the writeback has to occur under the inode lock.

Also relax the writeback requirement on the source, and rely on the
stateid checking to tell us if the source rebooted.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs42proc.c | 9 +++++++++
fs/nfs/nfs4file.c | 14 +-------------
2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 0f9f536e647b..778ad2778191 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -156,11 +156,20 @@ static ssize_t _nfs42_proc_copy(struct file *src, loff_t pos_src,
if (status)
return status;

+ status = filemap_write_and_wait_range(file_inode(src)->i_mapping,
+ pos_src, pos_src + (loff_t)count - 1);
+ if (status)
+ return status;
+
status = nfs4_set_rw_stateid(&args.dst_stateid, dst_lock->open_context,
dst_lock, FMODE_WRITE);
if (status)
return status;

+ status = nfs_sync_inode(dst_inode);
+ if (status)
+ return status;
+
status = nfs4_call_sync(server->client, server, &msg,
&args.seq_args, &res.seq_res, 0);
if (status == -ENOTSUPP)
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 014b0e41ace5..7cdc0ab9e6f5 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -133,21 +133,9 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
size_t count, unsigned int flags)
{
- struct inode *in_inode = file_inode(file_in);
- struct inode *out_inode = file_inode(file_out);
- int ret;
-
- if (in_inode == out_inode)
+ if (file_inode(file_in) == file_inode(file_out))
return -EINVAL;

- /* flush any pending writes */
- ret = nfs_sync_inode(in_inode);
- if (ret)
- return ret;
- ret = nfs_sync_inode(out_inode);
- if (ret)
- return ret;
-
return nfs42_proc_copy(file_in, pos_in, file_out, pos_out, count);
}

--
2.7.4


2016-06-26 00:26:43

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 4/5] NFS: Getattr doesn't require data sync semantics

When retrieving stat() information, NFS unfortunately does require us to
sync writes to disk in order to ensure that mtime and ctime are up to
date. However we shouldn't have to ensure that those writes are persisted.

Relaxing that requirement does mean that we may see an mtime/ctime change
if the server reboots and forces us to replay all writes.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/inode.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index b729f7b972d3..8eb2d96fd45b 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -661,9 +661,7 @@ int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
trace_nfs_getattr_enter(inode);
/* Flush out writes to the server in order to update c/mtime. */
if (S_ISREG(inode->i_mode)) {
- inode_lock(inode);
- err = nfs_sync_inode(inode);
- inode_unlock(inode);
+ err = filemap_write_and_wait(inode->i_mapping);
if (err)
goto out;
}
--
2.7.4


2016-06-26 00:26:43

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 5/5] NFS: llseek(SEEK_HOLE) and llseek(SEEK_DATA) don't require data sync

We want to ensure that we write the cached data to the server, but
don't require it be synced to disk. If the server reboots, we will
get a stateid error, which will cause us to retry anyway.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs42proc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 778ad2778191..531f7d5b6803 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -269,7 +269,11 @@ static loff_t _nfs42_proc_llseek(struct file *filep,
if (status)
return status;

- nfs_wb_all(inode);
+ status = filemap_write_and_wait_range(inode->i_mapping,
+ offset, LLONG_MAX);
+ if (status)
+ return status;
+
status = nfs4_call_sync(server->client, server, &msg,
&args.seq_args, &res.seq_res, 0);
if (status == -ENOTSUPP)
--
2.7.4