From 825dc41ad4bf262e3d8e6ccb5d549709bf1e53c5 Mon Sep 17 00:00:00 2001 From: Ameer Hamza Date: Sun, 4 Jan 2026 21:10:02 +0500 Subject: [PATCH] L2ARC: Preserve L2HDR in arc_release() for in-flight writes When arc_release() is called on a header with a single buffer and L2_WRITING set, the L2HDR must be preserved for ABD cleanup (similar to the arc_hdr_destroy() case). If we destroy the L2HDR here, later arc_write() will allocate a new ABD and call arc_hdr_free_abd(), which needs b_l2hdr.b_dev to properly defer ABD cleanup, causing VERIFY(HDR_HAS_L2HDR(hdr)) to fail. Allocate a new header for the buffer in the single_buf_l2writing case (single buffer + L2_WRITING), leaving the original header with L2HDR intact. The original header becomes an "orphan" (no buffers, no b_pabd) but retains device association for ABD cleanup when l2arc_write_done() completes. The shared buffer case (HDR_SHARED_DATA) is excluded because L2ARC makes its own transformed copy via l2arc_apply_transforms(), so the original ABD is not used by the L2 write. The header can be safely reused without allocating a new one. For proper evictable space accounting, arc_buf_remove() must be called before remove_reference() in the single_buf_l2writing path. This ensures arc_evictable_space_increment() (during remove_reference) and arc_evictable_space_decrement() (during destruction) see the same state (b_buf=NULL), preventing accounting leaks that cause module unload to hang with non-zero esize. Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Signed-off-by: Ameer Hamza Closes #18093 --- module/zfs/arc.c | 88 ++++++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/module/zfs/arc.c b/module/zfs/arc.c index aa193fe59..5b0335e67 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -6667,16 +6667,22 @@ arc_release(arc_buf_t *buf, const void *tag) ASSERT3S(zfs_refcount_count(&hdr->b_l1hdr.b_refcnt), >, 0); /* - * Do we have more than one buf? + * Do we have more than one buf? Or L2_WRITING with unshared data? + * Single-buf L2_WRITING with shared data can reuse the header since + * L2ARC uses its own transformed copy. */ - if (hdr->b_l1hdr.b_buf != buf || !ARC_BUF_LAST(buf)) { + if (hdr->b_l1hdr.b_buf != buf || !ARC_BUF_LAST(buf) || + (HDR_L2_WRITING(hdr) && !ARC_BUF_SHARED(buf))) { arc_buf_hdr_t *nhdr; uint64_t spa = hdr->b_spa; uint64_t psize = HDR_GET_PSIZE(hdr); uint64_t lsize = HDR_GET_LSIZE(hdr); boolean_t protected = HDR_PROTECTED(hdr); enum zio_compress compress = arc_hdr_get_compress(hdr); + uint8_t complevel = hdr->b_complevel; arc_buf_contents_t type = arc_buf_type(hdr); + boolean_t single_buf_l2writing = (hdr->b_l1hdr.b_buf == buf && + ARC_BUF_LAST(buf) && HDR_L2_WRITING(hdr)); if (ARC_BUF_SHARED(buf) && !ARC_BUF_COMPRESSED(buf)) { ASSERT3P(hdr->b_l1hdr.b_buf, !=, buf); @@ -6687,48 +6693,51 @@ arc_release(arc_buf_t *buf, const void *tag) * Pull the buffer off of this hdr and find the last buffer * in the hdr's buffer list. */ - VERIFY3S(remove_reference(hdr, tag), >, 0); arc_buf_t *lastbuf = arc_buf_remove(hdr, buf); - ASSERT3P(lastbuf, !=, NULL); + EQUIV(single_buf_l2writing, lastbuf == NULL); /* * If the current arc_buf_t and the hdr are sharing their data * buffer, then we must stop sharing that block. */ - if (ARC_BUF_SHARED(buf)) { - ASSERT(!arc_buf_is_shared(lastbuf)); + if (!single_buf_l2writing) { + if (ARC_BUF_SHARED(buf)) { + ASSERT(!arc_buf_is_shared(lastbuf)); - /* - * First, sever the block sharing relationship between - * buf and the arc_buf_hdr_t. - */ - arc_unshare_buf(hdr, buf); + /* + * First, sever the block sharing relationship + * between buf and the arc_buf_hdr_t. + */ + arc_unshare_buf(hdr, buf); - /* - * Now we need to recreate the hdr's b_pabd. Since we - * have lastbuf handy, we try to share with it, but if - * we can't then we allocate a new b_pabd and copy the - * data from buf into it. - */ - if (arc_can_share(hdr, lastbuf)) { - arc_share_buf(hdr, lastbuf); - } else { - arc_hdr_alloc_abd(hdr, 0); - abd_copy_from_buf(hdr->b_l1hdr.b_pabd, - buf->b_data, psize); + /* + * Now we need to recreate the hdr's b_pabd. + * Since we have lastbuf handy, we try to share + * with it, but if we can't then we allocate a + * new b_pabd and copy the data from buf into it + */ + if (arc_can_share(hdr, lastbuf)) { + arc_share_buf(hdr, lastbuf); + } else { + arc_hdr_alloc_abd(hdr, 0); + abd_copy_from_buf(hdr->b_l1hdr.b_pabd, + buf->b_data, psize); + } + } else if (HDR_SHARED_DATA(hdr)) { + /* + * Uncompressed shared buffers are always at the + * end of the list. Compressed buffers don't + * have the same requirements. This makes it + * hard to simply assert that the lastbuf is + * shared so we rely on the hdr's compression + * flags to determine if we have a compressed, + * shared buffer. + */ + ASSERT(arc_buf_is_shared(lastbuf) || + arc_hdr_get_compress(hdr) != + ZIO_COMPRESS_OFF); + ASSERT(!arc_buf_is_shared(buf)); } - } else if (HDR_SHARED_DATA(hdr)) { - /* - * Uncompressed shared buffers are always at the end - * of the list. Compressed buffers don't have the - * same requirements. This makes it hard to - * simply assert that the lastbuf is shared so - * we rely on the hdr's compression flags to determine - * if we have a compressed, shared buffer. - */ - ASSERT(arc_buf_is_shared(lastbuf) || - arc_hdr_get_compress(hdr) != ZIO_COMPRESS_OFF); - ASSERT(!arc_buf_is_shared(buf)); } ASSERT(hdr->b_l1hdr.b_pabd != NULL || HDR_HAS_RABD(hdr)); @@ -6743,10 +6752,15 @@ arc_release(arc_buf_t *buf, const void *tag) if (!arc_hdr_has_uncompressed_buf(hdr)) arc_cksum_free(hdr); + if (single_buf_l2writing) + VERIFY3S(remove_reference(hdr, tag), ==, 0); + else + VERIFY3S(remove_reference(hdr, tag), >, 0); + mutex_exit(hash_lock); - nhdr = arc_hdr_alloc(spa, psize, lsize, protected, - compress, hdr->b_complevel, type); + nhdr = arc_hdr_alloc(spa, psize, lsize, protected, compress, + complevel, type); ASSERT0P(nhdr->b_l1hdr.b_buf); ASSERT0(zfs_refcount_count(&nhdr->b_l1hdr.b_refcnt)); VERIFY3U(nhdr->b_type, ==, type);