From: George Wilson 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 Reviewed-by: Brian Behlendorf Signed-off-by: George Wilson 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); /*