From b8610c3d93fec2297a9f69fcab3f5b35f01e941f Mon Sep 17 00:00:00 2001 From: Ameer Hamza Date: Wed, 10 Sep 2025 20:20:13 +0500 Subject: [PATCH] L2ARC: Reorder header destruction for in-flight L2 writes With multiple L2ARC devices, headers can be destroyed asynchronously (e.g., during zpool sync) while L2_WRITING is set. The original code destroyed L2HDR before L1HDR, causing ABDs to lose their device association (b_l2hdr.b_dev) when arc_hdr_free_abd() is called. This caused ABDs to be added to the global free-on-write list without device information. When any L2ARC device completed its write and attempted to free these orphaned ABDs, it would panic on ASSERT(!list_link_active(&abd->abd_gang_link)) because the ABD was still part of another device's vdev_queue I/O aggregation gang. Fix by extending l2ad_mtx lock scope to cover L1HDR destruction and reordering to destroy L1HDR before L2HDR when L2_WRITING is set. This ensures arc_hdr_free_abd() can access b_l2hdr.b_dev to properly tag ABDs with their device for deferred cleanup. Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Signed-off-by: Ameer Hamza Closes #18093 --- module/zfs/arc.c | 55 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 611b50c1e..aa193fe59 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -3654,7 +3654,13 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr) } ASSERT(!HDR_IO_IN_PROGRESS(hdr)); ASSERT(!HDR_IN_HASH_TABLE(hdr)); + boolean_t l1hdr_destroyed = B_FALSE; + /* + * If L2_WRITING, destroy L1HDR before L2HDR (under mutex) so + * arc_hdr_free_abd() can properly defer ABDs. Otherwise, destroy + * L1HDR outside mutex to minimize contention. + */ if (HDR_HAS_L2HDR(hdr)) { l2arc_dev_t *dev = hdr->b_l2hdr.b_dev; boolean_t buflist_held = MUTEX_HELD(&dev->l2ad_mtx); @@ -3672,9 +3678,26 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr) * want to re-destroy the header's L2 portion. */ if (HDR_HAS_L2HDR(hdr)) { + if (HDR_L2_WRITING(hdr)) { + l1hdr_destroyed = B_TRUE; - if (!HDR_EMPTY(hdr)) - buf_discard_identity(hdr); + if (!HDR_EMPTY(hdr)) + buf_discard_identity(hdr); + + if (HDR_HAS_L1HDR(hdr)) { + arc_cksum_free(hdr); + + while (hdr->b_l1hdr.b_buf != NULL) + arc_buf_destroy_impl( + hdr->b_l1hdr.b_buf); + + if (hdr->b_l1hdr.b_pabd != NULL) + arc_hdr_free_abd(hdr, B_FALSE); + + if (HDR_HAS_RABD(hdr)) + arc_hdr_free_abd(hdr, B_TRUE); + } + } arc_hdr_l2hdr_destroy(hdr); } @@ -3683,26 +3706,22 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr) mutex_exit(&dev->l2ad_mtx); } - /* - * The header's identify can only be safely discarded once it is no - * longer discoverable. This requires removing it from the hash table - * and the l2arc header list. After this point the hash lock can not - * be used to protect the header. - */ - if (!HDR_EMPTY(hdr)) - buf_discard_identity(hdr); + if (!l1hdr_destroyed) { + if (!HDR_EMPTY(hdr)) + buf_discard_identity(hdr); - if (HDR_HAS_L1HDR(hdr)) { - arc_cksum_free(hdr); + if (HDR_HAS_L1HDR(hdr)) { + arc_cksum_free(hdr); - while (hdr->b_l1hdr.b_buf != NULL) - arc_buf_destroy_impl(hdr->b_l1hdr.b_buf); + while (hdr->b_l1hdr.b_buf != NULL) + arc_buf_destroy_impl(hdr->b_l1hdr.b_buf); - if (hdr->b_l1hdr.b_pabd != NULL) - arc_hdr_free_abd(hdr, B_FALSE); + if (hdr->b_l1hdr.b_pabd != NULL) + arc_hdr_free_abd(hdr, B_FALSE); - if (HDR_HAS_RABD(hdr)) - arc_hdr_free_abd(hdr, B_TRUE); + if (HDR_HAS_RABD(hdr)) + arc_hdr_free_abd(hdr, B_TRUE); + } } ASSERT0P(hdr->b_hash_next);