Fix out-of-order ZIL txtype lost on hardlinked files

We should only call zil_remove_async when an object is removed. However,
in current implementation, it is called whenever TX_REMOVE is called. In
the case of hardlinked file, every unlink will generate TX_REMOVE and
causing operations to be dropped even when the object is not removed.

We fix this by only calling zil_remove_async when the file is fully
unlinked.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Prakash Surya <prakash.surya@delphix.com>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes #8769
Closes #9061

diff --git a/include/sys/zfs_znode.h b/include/sys/zfs_znode.h
index ef53684f739646baff231ebe62c22eef7036fad1..a0a3dd1ad1f21154909004bf2c4d2e668d8c972b 100644 (file)
--- a/include/sys/zfs_znode.h
+++ b/include/sys/zfs_znode.h
@@ -371,7 +371,7 @@ extern void zfs_log_create(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
 extern int zfs_log_create_txtype(zil_create_t, vsecattr_t *vsecp,
     vattr_t *vap);
 extern void zfs_log_remove(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
-    znode_t *dzp, char *name, uint64_t foid);
+    znode_t *dzp, char *name, uint64_t foid, boolean_t unlinked);
 #define        ZFS_NO_OBJECT   0       /* no object id */
 extern void zfs_log_link(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
     znode_t *dzp, znode_t *zp, char *name);

diff --git a/module/zfs/zfs_log.c b/module/zfs/zfs_log.c
index ad5b5cf30b1eb5b337ef7caeb29f52469501497d..622ce08acd27a0d657fa6cb489004fc3541f2a4c 100644 (file)
--- a/module/zfs/zfs_log.c
+++ b/module/zfs/zfs_log.c
@@ -380,12 +380,14 @@ zfs_log_create(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
        zil_itx_assign(zilog, itx, tx);
 }
 
+void zil_remove_async(zilog_t *zilog, uint64_t oid);
+
 /*
  * Handles both TX_REMOVE and TX_RMDIR transactions.
  */
 void
 zfs_log_remove(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
-    znode_t *dzp, char *name, uint64_t foid)
+    znode_t *dzp, char *name, uint64_t foid, boolean_t unlinked)
 {
        itx_t *itx;
        lr_remove_t *lr;
@@ -401,6 +403,17 @@ zfs_log_remove(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
 
        itx->itx_oid = foid;
 
+       /*
+        * Object ids can be re-instantiated in the next txg so
+        * remove any async transactions to avoid future leaks.
+        * This can happen if a fsync occurs on the re-instantiated
+        * object for a WR_INDIRECT or WR_NEED_COPY write, which gets
+        * the new file data and flushes a write record for the old object.
+        */
+       if (unlinked) {
+               ASSERT((txtype & ~TX_CI) == TX_REMOVE);
+               zil_remove_async(zilog, foid);
+       }
        zil_itx_assign(zilog, itx, tx);
 }
 

diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
index 595bebc78df7fd4df2500d1fd62e32477557a6b2..1ad6f1588cc22f7ff9b3862f913812d9e07d3116 100644 (file)
--- a/module/zfs/zfs_vnops.c
+++ b/module/zfs/zfs_vnops.c
@@ -1886,7 +1886,7 @@ top:
        txtype = TX_REMOVE;
        if (flags & FIGNORECASE)
                txtype |= TX_CI;
-       zfs_log_remove(zilog, tx, txtype, dzp, name, obj);
+       zfs_log_remove(zilog, tx, txtype, dzp, name, obj, unlinked);
 
        dmu_tx_commit(tx);
 out:
@@ -2219,7 +2219,8 @@ top:
                uint64_t txtype = TX_RMDIR;
                if (flags & FIGNORECASE)
                        txtype |= TX_CI;
-               zfs_log_remove(zilog, tx, txtype, dzp, name, ZFS_NO_OBJECT);
+               zfs_log_remove(zilog, tx, txtype, dzp, name, ZFS_NO_OBJECT,
+                   B_FALSE);
        }
 
        dmu_tx_commit(tx);

diff --git a/module/zfs/zil.c b/module/zfs/zil.c
index 71079c4a38e457cb192d8ad780ad4659b2c92e17..98678aa446550c3d5b80fe96fac79a37169e24b2 100644 (file)
--- a/module/zfs/zil.c
+++ b/module/zfs/zil.c
@@ -1875,7 +1875,7 @@ zil_aitx_compare(const void *x1, const void *x2)
 /*
  * Remove all async itx with the given oid.
  */
-static void
+void
 zil_remove_async(zilog_t *zilog, uint64_t oid)
 {
        uint64_t otxg, txg;
@@ -1927,16 +1927,6 @@ zil_itx_assign(zilog_t *zilog, itx_t *itx, dmu_tx_t *tx)
        itxg_t *itxg;
        itxs_t *itxs, *clean = NULL;
 
-       /*
-        * Object ids can be re-instantiated in the next txg so
-        * remove any async transactions to avoid future leaks.
-        * This can happen if a fsync occurs on the re-instantiated
-        * object for a WR_INDIRECT or WR_NEED_COPY write, which gets
-        * the new file data and flushes a write record for the old object.
-        */
-       if ((itx->itx_lr.lrc_txtype & ~TX_CI) == TX_REMOVE)
-               zil_remove_async(zilog, itx->itx_oid);
-
        /*
         * Ensure the data of a renamed file is committed before the rename.
         */

diff --git a/tests/zfs-tests/tests/functional/slog/slog_replay_fs.ksh b/tests/zfs-tests/tests/functional/slog/slog_replay_fs.ksh
index 5f281a756f1552d2e38d43ce3bc97e5f3334ef63..ea3f8451b9e38ea80ffef46e19d10e1f3989db46 100755 (executable)
--- a/tests/zfs-tests/tests/functional/slog/slog_replay_fs.ksh
+++ b/tests/zfs-tests/tests/functional/slog/slog_replay_fs.ksh
@@ -160,6 +160,14 @@ log_must attr -qs fileattr -V HelloWorld /$TESTPOOL/$TESTFS/xattr.file
 log_must attr -qs tmpattr -V HelloWorld /$TESTPOOL/$TESTFS/xattr.file
 log_must attr -qr tmpattr /$TESTPOOL/$TESTFS/xattr.file
 
+# TX_WRITE, TX_LINK, TX_REMOVE
+# Make sure TX_REMOVE won't affect TX_WRITE if file is not destroyed
+log_must dd if=/dev/urandom of=/$TESTPOOL/$TESTFS/link_and_unlink bs=128k \
+   count=8
+log_must ln /$TESTPOOL/$TESTFS/link_and_unlink \
+   /$TESTPOOL/$TESTFS/link_and_unlink.link
+log_must rm /$TESTPOOL/$TESTFS/link_and_unlink.link
+
 #
 # 4. Copy TESTFS to temporary location (TESTDIR/copy)
 #