Fix race in dnode_check_slots_free()

Currently, dnode_check_slots_free() works by checking dn->dn_type
in the dnode to determine if the dnode is reclaimable. However,
there is a small window of time between dnode_free_sync() in the
first call to dsl_dataset_sync() and when the useraccounting code
is run when the type is set DMU_OT_NONE, but the dnode is not yet
evictable. This patch adds the ability for dnodes to know which
txg they were last dirtied in and adds a check for this before
performing the reclaim.

This patch also corrects several instances when dn_dirty_link was
treated as a list_node_t when it is technically a multilist_node_t.

Fixes: openzfs#7147

Signed-off-by: Tom Caputi <[email protected]>

include/sys/dnode.h

@@ -332,6 +332,7 @@ struct dnode {
 	kcondvar_t dn_notxholds;
 	enum dnode_dirtycontext dn_dirtyctx;
 	uint8_t *dn_dirtyctx_firstset;		/* dbg: contents meaningless */
+	uint64_t dn_dirty_txg;			/* txg dnode was last dirtied */
 
 	/* protected by own devices */
 	refcount_t dn_tx_holds;
@@ -432,6 +433,9 @@ void dnode_evict_dbufs(dnode_t *dn);
 void dnode_evict_bonus(dnode_t *dn);
 void dnode_free_interior_slots(dnode_t *dn);
 
+#define	DNODE_IS_DIRTY(_dn)						\
+	((_dn)->dn_dirty_txg >= spa_syncing_txg((_dn)->dn_objset->os_spa))
+
 #define	DNODE_IS_CACHEABLE(_dn)						\
 	((_dn)->dn_objset->os_primary_cache == ZFS_CACHE_ALL ||		\
 	(DMU_OT_IS_METADATA((_dn)->dn_type) &&				\

module/zfs/dbuf.c

@@ -1814,6 +1814,9 @@ dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx)
 			    FTAG);
 		}
 	}
+
+	if (tx->tx_txg > dn->dn_dirty_txg)
+		dn->dn_dirty_txg = tx->tx_txg;
 	mutex_exit(&dn->dn_mtx);
 
 	if (db->db_blkid == DMU_SPILL_BLKID)

module/zfs/dmu.c

@@ -2284,7 +2284,7 @@ dmu_offset_next(objset_t *os, uint64_t object, boolean_t hole, uint64_t *off)
 	 * Check if dnode is dirty
 	 */
 	for (i = 0; i < TXG_SIZE; i++) {
-		if (list_link_active(&dn->dn_dirty_link[i])) {
+		if (multilist_link_active(&dn->dn_dirty_link[i])) {
 			clean = B_FALSE;
 			break;
 		}

module/zfs/dnode.c

@@ -139,7 +139,7 @@ dnode_cons(void *arg, void *unused, int kmflag)
 	bzero(&dn->dn_next_maxblkid[0], sizeof (dn->dn_next_maxblkid));
 
 	for (i = 0; i < TXG_SIZE; i++) {
-		list_link_init(&dn->dn_dirty_link[i]);
+		multilist_link_init(&dn->dn_dirty_link[i]);
 		dn->dn_free_ranges[i] = NULL;
 		list_create(&dn->dn_dirty_records[i],
 		    sizeof (dbuf_dirty_record_t),
@@ -188,7 +188,7 @@ dnode_dest(void *arg, void *unused)
 	ASSERT(!list_link_active(&dn->dn_link));
 
 	for (i = 0; i < TXG_SIZE; i++) {
-		ASSERT(!list_link_active(&dn->dn_dirty_link[i]));
+		ASSERT(!multilist_link_active(&dn->dn_dirty_link[i]));
 		ASSERT3P(dn->dn_free_ranges[i], ==, NULL);
 		list_destroy(&dn->dn_dirty_records[i]);
 		ASSERT0(dn->dn_next_nblkptr[i]);
@@ -614,7 +614,7 @@ dnode_allocate(dnode_t *dn, dmu_object_type_t ot, int blocksize, int ibs,
 		ASSERT0(dn->dn_rm_spillblk[i]);
 		ASSERT0(dn->dn_next_blksz[i]);
 		ASSERT0(dn->dn_next_maxblkid[i]);
-		ASSERT(!list_link_active(&dn->dn_dirty_link[i]));
+		ASSERT(!multilist_link_active(&dn->dn_dirty_link[i]));
 		ASSERT3P(list_head(&dn->dn_dirty_records[i]), ==, NULL);
 		ASSERT3P(dn->dn_free_ranges[i], ==, NULL);
 	}
@@ -1081,31 +1081,40 @@ dnode_set_slots(dnode_children_t *children, int idx, int slots, void *ptr)
 	}
 }
 
+static boolean_t
+dnode_check_evictable(dnode_t *dn)
+{
+	boolean_t can_evict;
+
+	mutex_enter(&dn->dn_mtx);
+	can_evict = (dn->dn_type == DMU_OT_NONE && !DNODE_IS_DIRTY(dn));
+	mutex_exit(&dn->dn_mtx);
+	return (can_evict);
+}
+
 static boolean_t
 dnode_check_slots_free(dnode_children_t *children, int idx, int slots)
 {
 	ASSERT3S(idx + slots, <=, DNODES_PER_BLOCK);
 
+	/*
+	 * If all dnode slots are either already free or
+	 * evictable return B_TRUE.
+	 */
 	for (int i = idx; i < idx + slots; i++) {
 		dnode_handle_t *dnh = &children->dnc_children[i];
 		dnode_t *dn = dnh->dnh_dnode;
 
 		if (dn == DN_SLOT_FREE) {
 			continue;
 		} else if (DN_SLOT_IS_PTR(dn)) {
-			mutex_enter(&dn->dn_mtx);
-			dmu_object_type_t type = dn->dn_type;
-			mutex_exit(&dn->dn_mtx);
-
-			if (type != DMU_OT_NONE)
+			if (!dnode_check_evictable(dn))
 				return (B_FALSE);
-
-			continue;
+			else
+				continue;
 		} else {
 			return (B_FALSE);
 		}
-
-		return (B_FALSE);
 	}
 
 	return (B_TRUE);
@@ -1633,7 +1642,7 @@ dnode_setdirty(dnode_t *dn, dmu_tx_t *tx)
 	/*
 	 * If we are already marked dirty, we're done.
 	 */
-	if (list_link_active(&dn->dn_dirty_link[txg & TXG_MASK])) {
+	if (multilist_link_active(&dn->dn_dirty_link[txg & TXG_MASK])) {
 		multilist_sublist_unlock(mls);
 		return;
 	}