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 <alexander.motin@TrueNAS.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ameer Hamza <ahamza@ixsystems.com>
Closes #18093
This commit is contained in:
Ameer Hamza 2025-09-10 20:20:13 +05:00 committed by Brian Behlendorf
parent 2f41b9d865
commit b8610c3d93

View File

@ -3654,7 +3654,13 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr)
} }
ASSERT(!HDR_IO_IN_PROGRESS(hdr)); ASSERT(!HDR_IO_IN_PROGRESS(hdr));
ASSERT(!HDR_IN_HASH_TABLE(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)) { if (HDR_HAS_L2HDR(hdr)) {
l2arc_dev_t *dev = hdr->b_l2hdr.b_dev; l2arc_dev_t *dev = hdr->b_l2hdr.b_dev;
boolean_t buflist_held = MUTEX_HELD(&dev->l2ad_mtx); 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. * want to re-destroy the header's L2 portion.
*/ */
if (HDR_HAS_L2HDR(hdr)) { if (HDR_HAS_L2HDR(hdr)) {
if (HDR_L2_WRITING(hdr)) {
l1hdr_destroyed = B_TRUE;
if (!HDR_EMPTY(hdr)) if (!HDR_EMPTY(hdr))
buf_discard_identity(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); arc_hdr_l2hdr_destroy(hdr);
} }
@ -3683,26 +3706,22 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr)
mutex_exit(&dev->l2ad_mtx); mutex_exit(&dev->l2ad_mtx);
} }
/* if (!l1hdr_destroyed) {
* The header's identify can only be safely discarded once it is no if (!HDR_EMPTY(hdr))
* longer discoverable. This requires removing it from the hash table buf_discard_identity(hdr);
* 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 (HDR_HAS_L1HDR(hdr)) { if (HDR_HAS_L1HDR(hdr)) {
arc_cksum_free(hdr); arc_cksum_free(hdr);
while (hdr->b_l1hdr.b_buf != NULL) while (hdr->b_l1hdr.b_buf != NULL)
arc_buf_destroy_impl(hdr->b_l1hdr.b_buf); arc_buf_destroy_impl(hdr->b_l1hdr.b_buf);
if (hdr->b_l1hdr.b_pabd != NULL) if (hdr->b_l1hdr.b_pabd != NULL)
arc_hdr_free_abd(hdr, B_FALSE); arc_hdr_free_abd(hdr, B_FALSE);
if (HDR_HAS_RABD(hdr)) if (HDR_HAS_RABD(hdr))
arc_hdr_free_abd(hdr, B_TRUE); arc_hdr_free_abd(hdr, B_TRUE);
}
} }
ASSERT0P(hdr->b_hash_next); ASSERT0P(hdr->b_hash_next);