From ac9d6a25e6cbb74776b959510493788f66f88b45 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Wed, 10 Oct 2018 09:50:32 +0900 Subject: Add zfs_refcount_transfer_ownership_many() When debugging is enabled and a zfs_refcount_t contains multiple holders using the same key, but different ref_counts, the wrong reference_t may be transferred. Add a zfs_refcount_transfer_ownership_many() function, like the existing zfs_refcount_*_many() functions, to match and transfer the correct refcount_t; This issue may occur when using encryption with refcount debugging enabled. An arc_buf_hdr_t can have references for both the hdr->b_l1hdr.b_pabd and hdr->b_crypt_hdr.b_rabd both of which use the hdr as the reference holder. When unsharing the buffer the p_abd should be transferred. This issue does not impact production builds because refcount holders are not tracked. Reviewed-by: Matthew Ahrens Signed-off-by: Tom Caputi Signed-off-by: Brian Behlendorf --- usr/src/uts/common/fs/zfs/arc.c | 6 ++++-- usr/src/uts/common/fs/zfs/refcount.c | 16 +++++++++++++++- usr/src/uts/common/fs/zfs/sys/refcount.h | 3 +++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/usr/src/uts/common/fs/zfs/arc.c b/usr/src/uts/common/fs/zfs/arc.c index e4faefe15e..1fe636fc8a 100644 --- a/usr/src/uts/common/fs/zfs/arc.c +++ b/usr/src/uts/common/fs/zfs/arc.c @@ -3238,7 +3238,8 @@ arc_share_buf(arc_buf_hdr_t *hdr, arc_buf_t *buf) * refcount ownership to the hdr since it always owns * the refcount whenever an arc_buf_t is shared. */ - zfs_refcount_transfer_ownership(&state->arcs_size, buf, hdr); + zfs_refcount_transfer_ownership_many(&hdr->b_l1hdr.b_state->arcs_size, + arc_hdr_size(hdr), buf, hdr); hdr->b_l1hdr.b_pabd = abd_get_from_buf(buf->b_data, arc_buf_size(buf)); abd_take_ownership_of_buf(hdr->b_l1hdr.b_pabd, HDR_ISTYPE_METADATA(hdr)); @@ -3268,7 +3269,8 @@ arc_unshare_buf(arc_buf_hdr_t *hdr, arc_buf_t *buf) * We are no longer sharing this buffer so we need * to transfer its ownership to the rightful owner. */ - zfs_refcount_transfer_ownership(&state->arcs_size, hdr, buf); + zfs_refcount_transfer_ownership_many(&hdr->b_l1hdr.b_state->arcs_size, + arc_hdr_size(hdr), hdr, buf); arc_hdr_clear_flags(hdr, ARC_FLAG_SHARED_DATA); abd_release_ownership_of_buf(hdr->b_l1hdr.b_pabd); abd_put(hdr->b_l1hdr.b_pabd); diff --git a/usr/src/uts/common/fs/zfs/refcount.c b/usr/src/uts/common/fs/zfs/refcount.c index cac716e469..5e1f793642 100644 --- a/usr/src/uts/common/fs/zfs/refcount.c +++ b/usr/src/uts/common/fs/zfs/refcount.c @@ -235,8 +235,13 @@ zfs_refcount_transfer(zfs_refcount_t *dst, zfs_refcount_t *src) } void +<<<<<<< HEAD zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder, void *new_holder) +======= +refcount_transfer_ownership_many(refcount_t *rc, uint64_t number, + void *current_holder, void *new_holder) +>>>>>>> 592888158f... Add zfs_refcount_transfer_ownership_many() { reference_t *ref; boolean_t found = B_FALSE; @@ -249,7 +254,8 @@ zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder, for (ref = list_head(&rc->rc_list); ref; ref = list_next(&rc->rc_list, ref)) { - if (ref->ref_holder == current_holder) { + if (ref->ref_holder == current_holder && + ref->ref_number == number) { ref->ref_holder = new_holder; found = B_TRUE; break; @@ -259,6 +265,14 @@ zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder, mutex_exit(&rc->rc_mtx); } +void +refcount_transfer_ownership(refcount_t *rc, void *current_holder, + void *new_holder) +{ + return (refcount_transfer_ownership_many(rc, 1, current_holder, + new_holder)); +} + /* * If tracking is enabled, return true if a reference exists that matches * the "holder" tag. If tracking is disabled, then return true if a reference diff --git a/usr/src/uts/common/fs/zfs/sys/refcount.h b/usr/src/uts/common/fs/zfs/sys/refcount.h index 0059a245ee..fd39d3d439 100644 --- a/usr/src/uts/common/fs/zfs/sys/refcount.h +++ b/usr/src/uts/common/fs/zfs/sys/refcount.h @@ -76,6 +76,8 @@ int64_t zfs_refcount_add_many(zfs_refcount_t *, uint64_t, void *); int64_t zfs_refcount_remove_many(zfs_refcount_t *, uint64_t, void *); void zfs_refcount_transfer(zfs_refcount_t *, zfs_refcount_t *); void zfs_refcount_transfer_ownership(zfs_refcount_t *, void *, void *); +void zfs_refcount_transfer_ownership_many(refcount_t *, uint64_t, + void *, void *); boolean_t zfs_refcount_held(zfs_refcount_t *, void *); boolean_t zfs_refcount_not_held(zfs_refcount_t *, void *); @@ -107,6 +109,7 @@ typedef struct refcount { atomic_add_64(&(dst)->rc_count, __tmp); \ } #define zfs_refcount_transfer_ownership(rc, current_holder, new_holder) (void)0 +#define zfs_refcount_transfer_ownership_many(rc, nr, ch, nh) ((void)0) #define zfs_refcount_held(rc, holder) ((rc)->rc_count > 0) #define zfs_refcount_not_held(rc, holder) (B_TRUE) -- cgit v1.2.3