zio: make io_bp_copy be a separate allocation

We only need our own copy of the BP if the caller provided one that they
would free before we're done with it.

Additionally, L2ARC currently abuses io_bp_copy to smuggle the
originally BP back to arc_read_done() inside the L2ARC read ZIO. A small
helper function is provided to handle that allocation when necessary.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>

Author: robn

include/sys/zio.h

@@ -486,7 +486,7 @@ struct zio {
 	spa_t		*io_spa;
 	blkptr_t	*io_bp;
 	blkptr_t	*io_bp_override;
-	blkptr_t	io_bp_copy;
+	blkptr_t	*io_bp_copy;
 	list_t		io_parent_list;
 	list_t		io_child_list;
 	zio_t		*io_logical;
@@ -687,6 +687,8 @@ extern void zio_resume_wait(spa_t *spa);
 extern int zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp,
     enum blk_config_flag blk_config, enum blk_verify_flag blk_verify);
 
+extern void zio_force_bp(zio_t *zio, const blkptr_t *bp);
+
 /*
  * Initial setup and teardown.
  */

module/zfs/arc.c

@@ -8964,8 +8964,7 @@ l2arc_read_done(zio_t *zio)
 	 */
 	ASSERT(zio->io_abd == hdr->b_l1hdr.b_pabd ||
 	    (HDR_HAS_RABD(hdr) && zio->io_abd == hdr->b_crypt_hdr.b_rabd));
-	zio->io_bp_copy = cb->l2rcb_bp;	/* XXX fix in L2ARC 2.0	*/
-	zio->io_bp = &zio->io_bp_copy;	/* XXX fix in L2ARC 2.0	*/
+	zio_force_bp(zio, &cb->l2rcb_bp);	/* XXX fix in L2ARC 2.0 */
 	zio->io_prop.zp_complevel = hdr->b_complevel;
 
 	valid_cksum = arc_cksum_is_equal(hdr, zio);

module/zfs/zio.c

@@ -992,8 +992,10 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
 	if (bp != NULL) {
 		if (type != ZIO_TYPE_WRITE ||
 		    zio->io_child_type == ZIO_CHILD_DDT) {
-			zio->io_bp_copy = *bp;
-			zio->io_bp = &zio->io_bp_copy;	/* so caller can free */
+			zio->io_bp_copy =
+			    kmem_cache_alloc(zio_bp_cache, KM_SLEEP);
+			*zio->io_bp_copy = *bp;
+			zio->io_bp = zio->io_bp_copy;	/* so caller can free */
 		} else {
 			zio->io_bp = (blkptr_t *)bp;
 		}
@@ -1054,9 +1056,30 @@ zio_destroy(zio_t *zio)
 	list_destroy(&zio->io_child_list);
 	mutex_destroy(&zio->io_lock);
 	cv_destroy(&zio->io_cv);
+	if (zio->io_bp_copy != NULL)
+		kmem_cache_free(zio_bp_cache, zio->io_bp_copy);
 	kmem_cache_free(zio_cache, zio);
 }
 
+/*
+ * Forcibly set the block pointer in a completed ZIO. This is only used from
+ * l2arc_read_done() to set the original BP on the L2ARC read ZIO so that it
+ * looks sensible when sent back to arc_read_done(). Shouldn't be used anywhere
+ * else. If that changes,
+ * that is, you do something about the "XXX fix in L2ARC 2.0" comment in arc.c
+ * that has existed since prehistory, please rewrite this comment too.
+ */
+void
+zio_force_bp(zio_t *zio, const blkptr_t *bp)
+{
+	ASSERT3U(zio->io_type, ==, ZIO_TYPE_READ);
+	ASSERT3U(zio->io_stage, ==, ZIO_STAGE_DONE);
+	ASSERT0P(zio->io_bp_copy);
+	zio->io_bp_copy = kmem_cache_alloc(zio_bp_cache, KM_SLEEP);
+	*zio->io_bp_copy = *bp;
+	zio->io_bp = zio->io_bp_copy;
+}
+
 /*
  * ZIO intended to be between others.  Provides synchronization at READY
  * and DONE pipeline stages and calls the respective callbacks.
@@ -1814,7 +1837,7 @@ zio_read_bp_init(zio_t *zio)
 	uint64_t psize =
 	    BP_IS_EMBEDDED(bp) ? BPE_GET_PSIZE(bp) : BP_GET_PSIZE(bp);
 
-	ASSERT3P(zio->io_bp, ==, &zio->io_bp_copy);
+	ASSERT3P(zio->io_bp, ==, zio->io_bp_copy);
 
 	if (BP_GET_COMPRESS(bp) != ZIO_COMPRESS_OFF &&
 	    zio->io_child_type == ZIO_CHILD_LOGICAL &&
@@ -2142,7 +2165,7 @@ zio_free_bp_init(zio_t *zio)
 			zio->io_pipeline = ZIO_DDT_FREE_PIPELINE;
 	}
 
-	ASSERT3P(zio->io_bp, ==, &zio->io_bp_copy);
+	ASSERT3P(zio->io_bp, ==, zio->io_bp_copy);
 
 	return (zio);
 }
@@ -5308,8 +5331,12 @@ zio_ready(zio_t *zio)
 	}
 
 #ifdef ZFS_DEBUG
-	if (bp != NULL && bp != &zio->io_bp_copy)
-		zio->io_bp_copy = *bp;
+	if (bp != NULL && bp != zio->io_bp_copy) {
+		if (zio->io_bp_copy == NULL)
+			zio->io_bp_copy =
+			    kmem_cache_alloc(zio_bp_cache, KM_SLEEP);
+		*zio->io_bp_copy = *bp;
+	}
 #endif
 
 	if (zio->io_error != 0) {
@@ -5455,9 +5482,11 @@ zio_done(zio_t *zio)
 			ASSERT0(zio->io_children[c][w]);
 
 	if (zio->io_bp != NULL && !BP_IS_EMBEDDED(zio->io_bp)) {
-		ASSERT(memcmp(zio->io_bp, &zio->io_bp_copy,
-		    sizeof (blkptr_t)) == 0 ||
-		    (zio->io_bp == zio_unique_parent(zio)->io_bp));
+		if (zio->io_bp_copy) {
+			ASSERT(memcmp(zio->io_bp, zio->io_bp_copy,
+			    sizeof (blkptr_t)) == 0 ||
+			    (zio->io_bp == zio_unique_parent(zio)->io_bp));
+		}
 		if (zio->io_type == ZIO_TYPE_WRITE && !BP_IS_HOLE(zio->io_bp) &&
 		    zio->io_bp_override == NULL &&
 		    !(zio->io_flags & ZIO_FLAG_IO_REPAIR)) {