Fix inode eviction and sync writeback deadlock on Linux

If inode eviction and sync writeback happen on the same inode at the
same time, inode eviction will set I_FREEING and wait for sync
writeback, and sync writeback may eventually calls zfs_get_data and loop
in zfs_zget forever because igrab cannot succeed with I_FREEING, thus
causing deadlock.

To fix this, we take a hold on znode in itx. This make sure the inode
won't get evicted until sync writeback is done.

Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Fixes openzfs#7964
Fixes openzfs#9430

Author: davidchenntnx

cmd/ztest.c

@@ -2487,7 +2487,7 @@ ztest_get_done(zgd_t *zgd, int error)
 }
 
 static int
-ztest_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
+ztest_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
     struct lwb *lwb, zio_t *zio)
 {
 	(void) arg2;

include/sys/zil.h

@@ -457,6 +457,7 @@ typedef enum {
 } itx_wr_state_t;
 
 typedef void (*zil_callback_t)(void *data, int err);
+typedef void (*itx_zrele_t)(void *);
 
 typedef struct itx {
 	list_node_t	itx_node;	/* linkage on zl_itx_list */
@@ -467,7 +468,8 @@ typedef struct itx {
 	void		*itx_callback_data; /* User data for the callback */
 	size_t		itx_size;	/* allocated itx structure size */
 	uint64_t	itx_oid;	/* object id */
-	uint64_t	itx_gen;	/* gen number for zfs_get_data */
+	void		*itx_znode;	/* znode for zfs_get_data */
+	itx_zrele_t	itx_zrele;	/* cleanup for itx_znode */
 	lr_t		itx_lr;		/* common part of log record */
 	uint8_t		itx_lr_data[];	/* type-specific part of lr_xx_t */
 } itx_t;
@@ -605,7 +607,7 @@ typedef int zil_parse_blk_func_t(zilog_t *zilog, const blkptr_t *bp, void *arg,
 typedef int zil_parse_lr_func_t(zilog_t *zilog, const lr_t *lr, void *arg,
     uint64_t txg);
 typedef int zil_replay_func_t(void *arg1, void *arg2, boolean_t byteswap);
-typedef int zil_get_data_t(void *arg, uint64_t arg2, lr_write_t *lr, char *dbuf,
+typedef int zil_get_data_t(void *arg, void *arg2, lr_write_t *lr, char *dbuf,
     struct lwb *lwb, zio_t *zio);
 
 extern int zil_parse(zilog_t *zilog, zil_parse_blk_func_t *parse_blk_func,

include/sys/zvol_impl.h

@@ -115,7 +115,7 @@ void zvol_log_truncate(zvol_state_t *zv, dmu_tx_t *tx, uint64_t off,
     uint64_t len);
 void zvol_log_write(zvol_state_t *zv, dmu_tx_t *tx, uint64_t offset,
     uint64_t size, boolean_t commit);
-int zvol_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
+int zvol_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
     struct lwb *lwb, zio_t *zio);
 int zvol_init_impl(void);
 void zvol_fini_impl(void);

module/os/freebsd/zfs/zfs_vfsops.c

@@ -1598,6 +1598,29 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
 	znode_t	*zp;
 	dsl_dir_t *dd;
 
+	ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);
+
+	if (!unmounting) {
+		/*
+		 * We purge the parent filesystem's vfsp as the parent
+		 * filesystem and all of its snapshots have their vnode's
+		 * v_vfsp set to the parent's filesystem's vfsp.  Note,
+		 * 'z_parent' is self referential for non-snapshots.
+		 */
+#ifdef FREEBSD_NAMECACHE
+		cache_purgevfs(zfsvfs->z_parent->z_vfs);
+#endif
+	}
+
+	/*
+	 * Close the zil. NB: Can't close the zil while zfs_inactive
+	 * threads are blocked as zil_close can call zfs_inactive.
+	 */
+	if (zfsvfs->z_log) {
+		zil_close(zfsvfs->z_log);
+		zfsvfs->z_log = NULL;
+	}
+
 	/*
 	 * If someone has not already unmounted this file system,
 	 * drain the zrele_taskq to ensure all active references to the
@@ -1624,28 +1647,6 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
 				break;
 		}
 	}
-	ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);
-
-	if (!unmounting) {
-		/*
-		 * We purge the parent filesystem's vfsp as the parent
-		 * filesystem and all of its snapshots have their vnode's
-		 * v_vfsp set to the parent's filesystem's vfsp.  Note,
-		 * 'z_parent' is self referential for non-snapshots.
-		 */
-#ifdef FREEBSD_NAMECACHE
-		cache_purgevfs(zfsvfs->z_parent->z_vfs);
-#endif
-	}
-
-	/*
-	 * Close the zil. NB: Can't close the zil while zfs_inactive
-	 * threads are blocked as zil_close can call zfs_inactive.
-	 */
-	if (zfsvfs->z_log) {
-		zil_close(zfsvfs->z_log);
-		zfsvfs->z_log = NULL;
-	}
 
 	ZFS_TEARDOWN_INACTIVE_ENTER_WRITE(zfsvfs);
 

module/os/linux/zfs/zfs_vfsops.c

@@ -1349,6 +1349,28 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
 
 	zfs_unlinked_drain_stop_wait(zfsvfs);
 
+	ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);
+
+	if (!unmounting) {
+		/*
+		 * We purge the parent filesystem's super block as the
+		 * parent filesystem and all of its snapshots have their
+		 * inode's super block set to the parent's filesystem's
+		 * super block.  Note,  'z_parent' is self referential
+		 * for non-snapshots.
+		 */
+		shrink_dcache_sb(zfsvfs->z_parent->z_sb);
+	}
+
+	/*
+	 * Close the zil. NB: Can't close the zil while zfs_inactive
+	 * threads are blocked as zil_close can call zfs_inactive.
+	 */
+	if (zfsvfs->z_log) {
+		zil_close(zfsvfs->z_log);
+		zfsvfs->z_log = NULL;
+	}
+
 	/*
 	 * If someone has not already unmounted this file system,
 	 * drain the zrele_taskq to ensure all active references to the
@@ -1376,28 +1398,6 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
 		}
 	}
 
-	ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);
-
-	if (!unmounting) {
-		/*
-		 * We purge the parent filesystem's super block as the
-		 * parent filesystem and all of its snapshots have their
-		 * inode's super block set to the parent's filesystem's
-		 * super block.  Note,  'z_parent' is self referential
-		 * for non-snapshots.
-		 */
-		shrink_dcache_sb(zfsvfs->z_parent->z_sb);
-	}
-
-	/*
-	 * Close the zil. NB: Can't close the zil while zfs_inactive
-	 * threads are blocked as zil_close can call zfs_inactive.
-	 */
-	if (zfsvfs->z_log) {
-		zil_close(zfsvfs->z_log);
-		zfsvfs->z_log = NULL;
-	}
-
 	rw_enter(&zfsvfs->z_teardown_inactive_lock, RW_WRITER);
 
 	/*

module/zfs/zfs_log.c

@@ -47,6 +47,7 @@
 #include <sys/spa.h>
 #include <sys/zfs_fuid.h>
 #include <sys/dsl_dataset.h>
+#include <sys/zfs_vnops.h>
 
 /*
  * These zfs_log_* functions must be called within a dmu tx, in one
@@ -615,7 +616,7 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,
 	dmu_buf_impl_t *db = (dmu_buf_impl_t *)sa_get_db(zp->z_sa_hdl);
 	uint32_t blocksize = zp->z_blksz;
 	itx_wr_state_t write_state;
-	uint64_t gen = 0, log_size = 0;
+	uint64_t log_size = 0;
 
 	if (zil_replaying(zilog, tx) || zp->z_unlinked ||
 	    zfs_xattr_owner_unlinked(zp)) {
@@ -627,9 +628,6 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,
 	write_state = zil_write_state(zilog, resid, blocksize, o_direct,
 	    commit);
 
-	(void) sa_lookup(zp->z_sa_hdl, SA_ZPL_GEN(ZTOZSB(zp)), &gen,
-	    sizeof (gen));
-
 	while (resid) {
 		itx_t *itx;
 		lr_write_t *lr;
@@ -683,7 +681,9 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,
 
 		itx->itx_private = ZTOZSB(zp);
 		itx->itx_sync = (zp->z_sync_cnt != 0);
-		itx->itx_gen = gen;
+		zhold(zp);
+		itx->itx_znode = zp;
+		itx->itx_zrele = (itx_zrele_t)zfs_zrele_async;
 
 		if (resid == len) {
 			itx->itx_callback = callback;

module/zfs/zfs_vnops.c

@@ -1326,45 +1326,27 @@ static void zfs_get_done(zgd_t *zgd, int error);
  * Get data to generate a TX_WRITE intent log record.
  */
 int
-zfs_get_data(void *arg, uint64_t gen, lr_write_t *lr, char *buf,
+zfs_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
     struct lwb *lwb, zio_t *zio)
 {
 	zfsvfs_t *zfsvfs = arg;
 	objset_t *os = zfsvfs->z_os;
-	znode_t *zp;
+	znode_t *zp = arg2;
 	uint64_t object = lr->lr_foid;
 	uint64_t offset = lr->lr_offset;
 	uint64_t size = lr->lr_length;
 	zgd_t *zgd;
 	int error = 0;
-	uint64_t zp_gen;
 
+	ASSERT3P(zp, !=, NULL);
 	ASSERT3P(lwb, !=, NULL);
 	ASSERT3U(size, !=, 0);
 
-	/*
-	 * Nothing to do if the file has been removed
-	 */
-	if (zfs_zget(zfsvfs, object, &zp) != 0)
-		return (SET_ERROR(ENOENT));
-	if (zp->z_unlinked) {
-		/*
-		 * Release the vnode asynchronously as we currently have the
-		 * txg stopped from syncing.
-		 */
-		zfs_zrele_async(zp);
+	if (zp->z_unlinked)
 		return (SET_ERROR(ENOENT));
-	}
-	/* check if generation number matches */
-	if (sa_lookup(zp->z_sa_hdl, SA_ZPL_GEN(zfsvfs), &zp_gen,
-	    sizeof (zp_gen)) != 0) {
-		zfs_zrele_async(zp);
-		return (SET_ERROR(EIO));
-	}
-	if (zp_gen != gen) {
-		zfs_zrele_async(zp);
-		return (SET_ERROR(ENOENT));
-	}
+
+	/* Hold zp ref for ourselves */
+	zhold(zp);
 
 	zgd = kmem_zalloc(sizeof (zgd_t), KM_SLEEP);
 	zgd->zgd_lwb = lwb;

module/zfs/zil.c

@@ -2457,7 +2457,7 @@ zil_lwb_commit(zilog_t *zilog, lwb_t *lwb, itx_t *itx)
 			 * writing completion before the vdev cache flushing.
 			 */
 			error = zilog->zl_get_data(itx->itx_private,
-			    itx->itx_gen, lrwb, dbuf, lwb,
+			    itx->itx_znode, lrwb, dbuf, lwb,
 			    lwb->lwb_child_zio);
 			if (dbuf != NULL && error == 0) {
 				/* Zero any padding bytes in the last block. */
@@ -2565,6 +2565,7 @@ zil_itx_clone(itx_t *oitx)
 	memcpy(itx, oitx, oitx->itx_size);
 	itx->itx_callback = NULL;
 	itx->itx_callback_data = NULL;
+	itx->itx_zrele = NULL;
 	return (itx);
 }
 
@@ -2580,6 +2581,11 @@ zil_itx_destroy(itx_t *itx, int err)
 	if (itx->itx_callback != NULL)
 		itx->itx_callback(itx->itx_callback_data, err);
 
+	if (itx->itx_zrele) {
+		ASSERT3P(itx->itx_znode, !=, NULL);
+		itx->itx_zrele(itx->itx_znode);
+	}
+
 	zio_data_buf_free(itx, itx->itx_size);
 }
 

module/zfs/zvol.c

@@ -972,7 +972,7 @@ zvol_get_done(zgd_t *zgd, int error)
  * Get data to generate a TX_WRITE intent log record.
  */
 int
-zvol_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
+zvol_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
     struct lwb *lwb, zio_t *zio)
 {
 	zvol_state_t *zv = arg;