From: George Wilson <gwilson@zfsmail.com>
Date: Fri, 22 Mar 2019 20:09:11 +0000 (-0400)
Subject: ZFS Reads may result in unneccesary calls to zil_commit
X-Git-Tag: zfs-0.8.0-rc4~53
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2efea7c82c5741a50b476bb1dcbc9a74b8f73ad1;p=zfs

ZFS Reads may result in unneccesary calls to zil_commit

ZFS supports O_RSYNC for read operations and when specified will ensure
the same level of data integrity that O_DSYNC and O_SYNC provides for
writes. O_RSYNC by itself has no effect so it must be combined with
either O_DSYNC or O_SYNC. However, many platforms don't support O_RSYNC
and have mapped O_SYNC to mean O_RSYNC within ZFS. This is incorrect
and causes unnecessary calls to zil_commit. Only platforms which
support O_RSYNC should implement the zil_commit functionality in the
read code path.

Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Wilson <george.wilson@delphix.com>
Closes #8523
---

diff --git a/include/spl/sys/vnode.h b/include/spl/sys/vnode.h
index 2c038a6d2..71278b08c 100644
--- a/include/spl/sys/vnode.h
+++ b/include/spl/sys/vnode.h
@@ -58,7 +58,6 @@
 #define	FOFFMAX		O_LARGEFILE
 #define	FSYNC		O_SYNC
 #define	FDSYNC		O_DSYNC
-#define	FRSYNC		O_SYNC
 #define	FEXCL		O_EXCL
 #define	FDIRECT		O_DIRECT
 #define	FAPPEND		O_APPEND
diff --git a/lib/libspl/include/sys/file.h b/lib/libspl/include/sys/file.h
index b5d985bda..e0752ac25 100644
--- a/lib/libspl/include/sys/file.h
+++ b/lib/libspl/include/sys/file.h
@@ -40,7 +40,6 @@
 #define	FOFFMAX	O_LARGEFILE
 #define	FSYNC	O_SYNC
 #define	FDSYNC	O_DSYNC
-#define	FRSYNC	O_RSYNC
 #define	FEXCL	O_EXCL
 
 #define	FNODSYNC	0x10000	/* fsync pseudo flag */
diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
index bbc171bd2..c77101485 100644
--- a/module/zfs/zfs_vnops.c
+++ b/module/zfs/zfs_vnops.c
@@ -439,6 +439,7 @@ int
 zfs_read(struct inode *ip, uio_t *uio, int ioflag, cred_t *cr)
 {
 	int error = 0;
+	boolean_t frsync = B_FALSE;
 
 	znode_t *zp = ITOZ(ip);
 	zfsvfs_t *zfsvfs = ITOZSB(ip);
@@ -466,12 +467,19 @@ zfs_read(struct inode *ip, uio_t *uio, int ioflag, cred_t *cr)
 		return (0);
 	}
 
+#ifdef FRSYNC
 	/*
 	 * If we're in FRSYNC mode, sync out this znode before reading it.
 	 * Only do this for non-snapshots.
+	 *
+	 * Some platforms do not support FRSYNC and instead map it
+	 * to FSYNC, which results in unnecessary calls to zil_commit. We
+	 * only honor FRSYNC requests on platforms which support it.
 	 */
+	frsync = !!(ioflag & FRSYNC);
+#endif
 	if (zfsvfs->z_log &&
-	    (ioflag & FRSYNC || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS))
+	    (frsync || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS))
 		zil_commit(zfsvfs->z_log, zp->z_id);
 
 	/*