Re: [PATCH v4 24/28] NFS: Getattr doesn't require data sync semantics

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jul 06, 2016 at 06:30:01PM -0400, Trond Myklebust wrote:
> 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.
> 
> The exception to this rule are pNFS clients that are required to send
> layoutcommit, however that is dealt with by the call to pnfs_sync_inode()
> in _nfs_revalidate_inode().

This one breaks xfstests generic/207 on block/scsi layout for me.  The
reason for that is that we need a layoutcommit after writing out all
data for the file for the file size to be updated on the server.

Below is my attempt to fix this by re-adding pnfs_sync_inode to
nfs_getattr.  The call in _nfs_revalidate_inode isn't enough as it
doesn't get called in most cases we care about.

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 22a53ee..8bd04cf 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -660,11 +660,20 @@ int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
 	int err = 0;
 
 	trace_nfs_getattr_enter(inode);
-	/* Flush out writes to the server in order to update c/mtime.  */
+
+	/*
+	 * Flush out writes to the server in order to update c/mtime as well
+	 * as the file size.  In the pNFS case this also requires a
+	 * LAYOUTCOMMIT.
+	 */
 	if (S_ISREG(inode->i_mode)) {
 		err = filemap_write_and_wait(inode->i_mapping);
 		if (err)
 			goto out;
+
+		err = pnfs_sync_inode(inode, true);
+		if (err)
+			goto out;
 	}
 
 	/*