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 2c038a6d291b4b6885f767f6570d13550544ee08..71278b08c86716a5bcb1cf2ca93affa85d41510c 100644 (file)
--- 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 b5d985bda264538327649479874784093dc7b420..e0752ac25c2bf7f71b63ca9e66c2474427c763dc 100644 (file)
--- 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 bbc171bd2830ac1a07483ecd56e6ea50c2c8adde..c77101485ffa53b0ebc9b0ad8c9ec17a01925647 100644 (file)
--- 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);
 
        /*