Skip to content

Commit 32e8a43

Browse files
committed
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]>
1 parent b22d109 commit 32e8a43

File tree

3 files changed

+38
-12
lines changed

3 files changed

+38
-12
lines changed

include/sys/zio.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ struct zio {
486486
spa_t *io_spa;
487487
blkptr_t *io_bp;
488488
blkptr_t *io_bp_override;
489-
blkptr_t io_bp_copy;
489+
blkptr_t *io_bp_copy;
490490
list_t io_parent_list;
491491
list_t io_child_list;
492492
zio_t *io_logical;
@@ -688,6 +688,8 @@ extern void zio_resume_wait(spa_t *spa);
688688
extern int zfs_blkptr_verify(spa_t *spa, const blkptr_t *bp,
689689
enum blk_config_flag blk_config, enum blk_verify_flag blk_verify);
690690

691+
extern void zio_force_bp(zio_t *zio, const blkptr_t *bp);
692+
691693
/*
692694
* Initial setup and teardown.
693695
*/

module/zfs/arc.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8964,8 +8964,7 @@ l2arc_read_done(zio_t *zio)
89648964
*/
89658965
ASSERT(zio->io_abd == hdr->b_l1hdr.b_pabd ||
89668966
(HDR_HAS_RABD(hdr) && zio->io_abd == hdr->b_crypt_hdr.b_rabd));
8967-
zio->io_bp_copy = cb->l2rcb_bp; /* XXX fix in L2ARC 2.0 */
8968-
zio->io_bp = &zio->io_bp_copy; /* XXX fix in L2ARC 2.0 */
8967+
zio_force_bp(zio, &cb->l2rcb_bp); /* XXX fix in L2ARC 2.0 */
89698968
zio->io_prop.zp_complevel = hdr->b_complevel;
89708969

89718970
valid_cksum = arc_cksum_is_equal(hdr, zio);

module/zfs/zio.c

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,8 +1000,8 @@ zio_create(zio_t *pio, spa_t *spa, uint64_t txg, const blkptr_t *bp,
10001000
if (bp != NULL) {
10011001
if (type != ZIO_TYPE_WRITE ||
10021002
zio->io_child_type == ZIO_CHILD_DDT) {
1003-
zio->io_bp_copy = *bp;
1004-
zio->io_bp = &zio->io_bp_copy; /* so caller can free */
1003+
/* so caller can free */
1004+
zio->io_bp = zio->io_bp_copy = zio_dup_bp(bp);
10051005
} else {
10061006
zio->io_bp = (blkptr_t *)bp;
10071007
}
@@ -1062,9 +1062,28 @@ zio_destroy(zio_t *zio)
10621062
list_destroy(&zio->io_child_list);
10631063
mutex_destroy(&zio->io_lock);
10641064
cv_destroy(&zio->io_cv);
1065+
if (zio->io_bp_copy != NULL)
1066+
kmem_cache_free(zio_bp_cache, zio->io_bp_copy);
10651067
kmem_cache_free(zio_cache, zio);
10661068
}
10671069

1070+
/*
1071+
* Forcibly set the block pointer in a completed ZIO. This is only used from
1072+
* l2arc_read_done() to set the original BP on the L2ARC read ZIO so that it
1073+
* looks sensible when sent back to arc_read_done(). Shouldn't be used anywhere
1074+
* else. If that changes, that is, you do something about the "XXX fix in L2ARC
1075+
* 2.0" comment in arc.c that has existed since prehistory, please rewrite this
1076+
* comment too.
1077+
*/
1078+
void
1079+
zio_force_bp(zio_t *zio, const blkptr_t *bp)
1080+
{
1081+
ASSERT3U(zio->io_type, ==, ZIO_TYPE_READ);
1082+
ASSERT3U(zio->io_stage, ==, ZIO_STAGE_DONE);
1083+
ASSERT0P(zio->io_bp_copy);
1084+
zio->io_bp = zio->io_bp_copy = zio_dup_bp(bp);
1085+
}
1086+
10681087
/*
10691088
* ZIO intended to be between others. Provides synchronization at READY
10701089
* and DONE pipeline stages and calls the respective callbacks.
@@ -1822,7 +1841,7 @@ zio_read_bp_init(zio_t *zio)
18221841
uint64_t psize =
18231842
BP_IS_EMBEDDED(bp) ? BPE_GET_PSIZE(bp) : BP_GET_PSIZE(bp);
18241843

1825-
ASSERT3P(zio->io_bp, ==, &zio->io_bp_copy);
1844+
ASSERT3P(zio->io_bp, ==, zio->io_bp_copy);
18261845

18271846
if (BP_GET_COMPRESS(bp) != ZIO_COMPRESS_OFF &&
18281847
zio->io_child_type == ZIO_CHILD_LOGICAL &&
@@ -2150,7 +2169,7 @@ zio_free_bp_init(zio_t *zio)
21502169
zio->io_pipeline = ZIO_DDT_FREE_PIPELINE;
21512170
}
21522171

2153-
ASSERT3P(zio->io_bp, ==, &zio->io_bp_copy);
2172+
ASSERT3P(zio->io_bp, ==, zio->io_bp_copy);
21542173

21552174
return (zio);
21562175
}
@@ -5327,8 +5346,12 @@ zio_ready(zio_t *zio)
53275346
}
53285347

53295348
#ifdef ZFS_DEBUG
5330-
if (bp != NULL && bp != &zio->io_bp_copy)
5331-
zio->io_bp_copy = *bp;
5349+
if (bp != NULL && bp != zio->io_bp_copy) {
5350+
if (zio->io_bp_copy == NULL)
5351+
zio->io_bp_copy = zio_dup_bp(bp);
5352+
else
5353+
*zio->io_bp_copy = *bp;
5354+
}
53325355
#endif
53335356

53345357
if (zio->io_error != 0) {
@@ -5474,9 +5497,11 @@ zio_done(zio_t *zio)
54745497
ASSERT0(zio->io_children[c][w]);
54755498

54765499
if (zio->io_bp != NULL && !BP_IS_EMBEDDED(zio->io_bp)) {
5477-
ASSERT(memcmp(zio->io_bp, &zio->io_bp_copy,
5478-
sizeof (blkptr_t)) == 0 ||
5479-
(zio->io_bp == zio_unique_parent(zio)->io_bp));
5500+
if (zio->io_bp_copy) {
5501+
ASSERT(memcmp(zio->io_bp, zio->io_bp_copy,
5502+
sizeof (blkptr_t)) == 0 ||
5503+
(zio->io_bp == zio_unique_parent(zio)->io_bp));
5504+
}
54805505
if (zio->io_type == ZIO_TYPE_WRITE && !BP_IS_HOLE(zio->io_bp) &&
54815506
zio->io_bp_override == NULL &&
54825507
!(zio->io_flags & ZIO_FLAG_IO_REPAIR)) {

0 commit comments

Comments
 (0)