Handle interaction between gang blocks, copies, and FDT.

With the advent of fast dedup, there are no longer separate dedup tables
for different copies values. There is now logic that will add DVAs to
the dedup table entry if more copies are needed for new writes. However,
this interacts poorly with ganging. There are two different cases that
can result in mixed gang/non-gang BPs, which are illegal in ZFS.

This change forces updates of existing FDT entries to only add more DVAs
of the same type; if there are already gang DVAs in the FDT, we require
that the new allocations also be gangs. if there are non-gang DVAs in
the FDT, then this allocation may not be gangs. If it would gang, we
have to redo the whole write as a non-dedup write.

Sponsored by: iXsystems, Inc.
Sponsored-by: Klara, Inc.
Signed-off-by: Paul Dagnelie <[email protected]>

Author: Paul Dagnelie

include/sys/ddt.h

@@ -352,6 +352,8 @@ extern ddt_phys_variant_t ddt_phys_select(const ddt_t *ddt,
     const ddt_entry_t *dde, const blkptr_t *bp);
 extern uint64_t ddt_phys_birth(const ddt_univ_phys_t *ddp,
     ddt_phys_variant_t v);
+extern int ddt_phys_gang_count(const ddt_univ_phys_t *ddp,
+    ddt_phys_variant_t v, boolean_t encrypted);
 extern int ddt_phys_dva_count(const ddt_univ_phys_t *ddp, ddt_phys_variant_t v,
     boolean_t encrypted);
 

include/sys/zio.h

@@ -358,6 +358,7 @@ typedef struct zio_prop {
 	boolean_t		zp_encrypt;
 	boolean_t		zp_byteorder;
 	boolean_t		zp_direct_write;
+	boolean_t		zp_must_gang;
 	uint8_t			zp_salt[ZIO_DATA_SALT_LEN];
 	uint8_t			zp_iv[ZIO_DATA_IV_LEN];
 	uint8_t			zp_mac[ZIO_DATA_MAC_LEN];

module/zfs/ddt.c

@@ -814,6 +814,20 @@ ddt_phys_birth(const ddt_univ_phys_t *ddp, ddt_phys_variant_t v)
 		return (ddp->ddp_trad[v].ddp_phys_birth);
 }
 
+int
+ddt_phys_gang_count(const ddt_univ_phys_t *ddp, ddt_phys_variant_t v,
+    boolean_t encrypted)
+{
+	ASSERT3U(v, <, DDT_PHYS_NONE);
+
+	const dva_t *dvas = (v == DDT_PHYS_FLAT) ?
+	    ddp->ddp_flat.ddp_dva : ddp->ddp_trad[v].ddp_dva;
+
+	return (DVA_GET_GANG(&dvas[0]) +
+	    DVA_GET_GANG(&dvas[1] +
+	    DVA_GET_GANG(&dvas[2]) * !encrypted));
+}
+
 int
 ddt_phys_dva_count(const ddt_univ_phys_t *ddp, ddt_phys_variant_t v,
     boolean_t encrypted)

module/zfs/dmu.c

@@ -2493,6 +2493,7 @@ dmu_write_policy(objset_t *os, dnode_t *dn, int level, int wp, zio_prop_t *zp)
 	zp->zp_encrypt = encrypt;
 	zp->zp_byteorder = ZFS_HOST_BYTEORDER;
 	zp->zp_direct_write = (wp & WP_DIRECT_WR) ? B_TRUE : B_FALSE;
+	zp->zp_must_gang = B_FALSE;
 	memset(zp->zp_salt, 0, ZIO_DATA_SALT_LEN);
 	memset(zp->zp_iv, 0, ZIO_DATA_IV_LEN);
 	memset(zp->zp_mac, 0, ZIO_DATA_MAC_LEN);

module/zfs/zio.c

@@ -3138,6 +3138,17 @@ zio_write_gang_block(zio_t *pio, metaslab_class_t *mc)
 	 * encryption, which uses DVA[2] for the IV+salt.
 	 */
 	int gbh_copies = gio->io_prop.zp_gang_copies;
+	if (gbh_copies == 0) {
+		/*
+		 * This should only happen in the case where we're filling in
+		 * DDT entries for a parent that wants more copies than the DDT
+		 * has.  In that case, we cannot gang without creating a mixed
+		 * blkptr, which is illegal.
+		 */
+		ASSERT3U(gio->io_child_type, ==, ZIO_CHILD_DDT);
+		pio->io_error = EAGAIN;
+		return (pio);
+	}
 	ASSERT3S(gbh_copies, >, 0);
 	ASSERT3S(gbh_copies, <=, SPA_DVAS_PER_BP);
 
@@ -3149,7 +3160,7 @@ zio_write_gang_block(zio_t *pio, metaslab_class_t *mc)
 
 		flags |= METASLAB_ASYNC_ALLOC;
 		VERIFY(zfs_refcount_held(&mc->mc_allocator[pio->io_allocator].
-		    mca_alloc_slots, pio));
+		    mca_alloc_slots, pio) || gio->io_prop.zp_copies == 0);
 
 		/*
 		 * The logical zio has already placed a reservation for
@@ -3229,6 +3240,7 @@ zio_write_gang_block(zio_t *pio, metaslab_class_t *mc)
 		zp.zp_encrypt = gio->io_prop.zp_encrypt;
 		zp.zp_byteorder = gio->io_prop.zp_byteorder;
 		zp.zp_direct_write = B_FALSE;
+		zp.zp_must_gang = B_FALSE;
 		memset(zp.zp_salt, 0, ZIO_DATA_SALT_LEN);
 		memset(zp.zp_iv, 0, ZIO_DATA_IV_LEN);
 		memset(zp.zp_mac, 0, ZIO_DATA_MAC_LEN);
@@ -3784,8 +3796,9 @@ zio_ddt_write(zio_t *zio)
 	 */
 	int have_dvas = ddt_phys_dva_count(ddp, v, BP_IS_ENCRYPTED(bp));
 	IMPLY(have_dvas == 0, ddt_phys_birth(ddp, v) == 0);
+	int gang_dvas = ddt_phys_gang_count(ddp, v, BP_IS_ENCRYPTED(bp));
 
-	/* Number of DVAs requested bya the IO. */
+	/* Number of DVAs requested by the IO. */
 	uint8_t need_dvas = zp->zp_copies;
 
 	/*
@@ -3937,7 +3950,19 @@ zio_ddt_write(zio_t *zio)
 	 * grow the DDT entry by to satisfy the request.
 	 */
 	zio_prop_t czp = *zp;
-	czp.zp_copies = czp.zp_gang_copies = need_dvas;
+	if (gang_dvas > 0 && have_dvas > 0) {
+		czp.zp_gang_copies = need_dvas +
+		    (zp->zp_gang_copies - zp->zp_copies);
+		czp.zp_copies = need_dvas;
+		czp.zp_must_gang = B_TRUE;
+	} else if (gang_dvas == 0 && have_dvas > 0) {
+		czp.zp_copies = need_dvas;
+		czp.zp_gang_copies = 0;
+	} else {
+		czp.zp_copies = need_dvas;
+		czp.zp_gang_copies = need_dvas +
+		    (zp->zp_gang_copies - zp->zp_copies);
+	}
 	zio_t *cio = zio_write(zio, spa, txg, bp, zio->io_orig_abd,
 	    zio->io_orig_size, zio->io_orig_size, &czp,
 	    zio_ddt_child_write_ready, NULL,
@@ -4109,6 +4134,7 @@ zio_dva_allocate(zio_t *zio)
 	ASSERT(BP_IS_HOLE(bp));
 	ASSERT0(BP_GET_NDVAS(bp));
 	ASSERT3U(zio->io_prop.zp_copies, >, 0);
+
 	ASSERT3U(zio->io_prop.zp_copies, <=, spa_max_replication(spa));
 	ASSERT3U(zio->io_size, ==, BP_GET_PSIZE(bp));
 
@@ -4141,14 +4167,15 @@ zio_dva_allocate(zio_t *zio)
 	 * back to spa_sync() which is abysmal for performance.
 	 */
 	ASSERT(ZIO_HAS_ALLOCATOR(zio));
-	error = metaslab_alloc(spa, mc, zio->io_size, bp,
-	    zio->io_prop.zp_copies, zio->io_txg, NULL, flags,
-	    &zio->io_alloc_list, zio, zio->io_allocator);
+	error = zio->io_prop.zp_must_gang ? ENOSPC : metaslab_alloc(spa,
+	    mc, zio->io_size, bp, zio->io_prop.zp_copies, zio->io_txg, NULL,
+	    flags, &zio->io_alloc_list, zio, zio->io_allocator);
 
 	/*
 	 * Fallback to normal class when an alloc class is full
 	 */
-	if (error == ENOSPC && mc != spa_normal_class(spa)) {
+	if (error == ENOSPC && mc != spa_normal_class(spa) &&
+	    !zio->io_prop.zp_must_gang) {
 		/*
 		 * When the dedup or special class is spilling into the  normal
 		 * class, there can still be significant space available due
@@ -4200,7 +4227,8 @@ zio_dva_allocate(zio_t *zio)
 	}
 
 	if (error == ENOSPC && zio->io_size > SPA_MINBLOCKSIZE) {
-		if (zfs_flags & ZFS_DEBUG_METASLAB_ALLOC) {
+		if (zfs_flags & ZFS_DEBUG_METASLAB_ALLOC &&
+		    !zio->io_prop.zp_must_gang) {
 			zfs_dbgmsg("%s: metaslab allocation failure, "
 			    "trying ganging: zio %px, size %llu, error %d",
 			    spa_name(spa), zio, (u_longlong_t)zio->io_size,
@@ -5422,6 +5450,16 @@ zio_done(zio_t *zio)
 	}
 
 	if (zio->io_error && zio == zio->io_logical) {
+
+		/*
+		 * A DDT child tried to create a mixed gang/non-gang BP. We're
+		 * going to have to just retry as a non-dedup IO.
+		 */
+		if (zio->io_error == EAGAIN && IO_IS_ALLOCATING(zio) &&
+		    zio->io_prop.zp_dedup) {
+			zio->io_reexecute |= ZIO_REEXECUTE_NOW;
+			zio->io_prop.zp_dedup = B_FALSE;
+		}
 		/*
 		 * Determine whether zio should be reexecuted.  This will
 		 * propagate all the way to the root via zio_notify_parent().

tests/runfiles/common.run

@@ -724,7 +724,7 @@ tests = ['large_dnode_001_pos', 'large_dnode_003_pos', 'large_dnode_004_neg',
 tags = ['functional', 'features', 'large_dnode']
 
 [tests/functional/gang_blocks]
-tests = ['gang_blocks_redundant']
+tests = ['gang_blocks_redundant', 'gang_blocks_ddt_copies']
 tags = ['functional', 'gang_blocks']
 
 [tests/functional/grow]

tests/zfs-tests/tests/Makefile.am

@@ -1559,6 +1559,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/features/large_dnode/large_dnode_009_pos.ksh \
 	functional/features/large_dnode/setup.ksh \
 	functional/gang_blocks/cleanup.ksh \
+	functional/gang_blocks/gang_blocks_ddt_copies.ksh \
 	functional/gang_blocks/gang_blocks_redundant.ksh \
 	functional/gang_blocks/setup.ksh \
 	functional/grow/grow_pool_001_pos.ksh \

tests/zfs-tests/tests/functional/gang_blocks/gang_blocks_ddt_copies.ksh

@@ -0,0 +1,66 @@
+#!/bin/ksh
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2025 by Klara Inc.
+#
+
+#
+# Description:
+# Verify that mixed gang blocks and copies interact correctly in FDT
+#
+# Strategy:
+# 1. Store a block with copies = 1 in the DDT unganged.
+# 2. Add a new entry with copies = 2 that gangs, ensure it doesn't panic
+# 3. Store a block with copies = 1 in the DDT ganged.
+# 4. Add a new entry with copies = 3 that doesn't gang, ensure that it doesn't panic.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/gang_blocks/gang_blocks.kshlib
+
+log_assert "Verify that mixed gang blocks and copies interact correctly in FDT"
+
+save_tunable DEDUP_LOG_TXG_MAX
+
+function cleanup2
+{
+	zfs destroy $TESTPOOL/fs1
+	zfs destroy $TESTPOOL/fs2
+	restore_tunable DEDUP_LOG_TXG_MAX
+	cleanup
+}
+
+preamble
+log_onexit cleanup2
+
+log_must zpool create -f -o ashift=9 $TESTPOOL $DISKS
+log_must zfs create -o recordsize=64k -o dedup=on $TESTPOOL/fs1
+log_must zfs create -o recordsize=64k -o dedup=on -o copies=3 $TESTPOOL/fs2
+set_tunable32 DEDUP_LOG_TXG_MAX 1
+log_must dd if=/dev/urandom of=/$TESTPOOL/fs1/f1 bs=64k count=1
+log_must sync_pool $TESTPOOL
+set_tunable32 METASLAB_FORCE_GANGING 20000
+set_tunable32 METASLAB_FORCE_GANGING_PCT 100
+log_must dd if=/$TESTPOOL/fs1/f1 of=/$TESTPOOL/fs2/f1 bs=64k count=1
+log_must sync_pool $TESTPOOL
+
+log_must rm /$TESTPOOL/fs*/f1
+log_must sync_pool $TESTPOOL
+log_must dd if=/dev/urandom of=/$TESTPOOL/fs1/f1 bs=64k count=1
+log_must sync_pool $TESTPOOL
+log_must zdb -D $TESTPOOL
+set_tunable32 METASLAB_FORCE_GANGING_PCT 0
+log_must dd if=/$TESTPOOL/fs1/f1 of=/$TESTPOOL/fs2/f1 bs=64k count=1
+log_must sync_pool $TESTPOOL # This will panic, case 3
+
+log_pass "Verify that mixed gang blocks and copies interact correctly in FDT"