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 <mahrens@delphix.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #7219
Closes #8000
This commit is contained in:
Brian Behlendorf 2018-10-08 14:58:21 -07:00
parent 4cbde2ecbf
commit d7e4b30a67
3 changed files with 21 additions and 9 deletions

View File

@ -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 *); 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(zfs_refcount_t *, zfs_refcount_t *);
void zfs_refcount_transfer_ownership(zfs_refcount_t *, void *, void *); void zfs_refcount_transfer_ownership(zfs_refcount_t *, void *, void *);
void zfs_refcount_transfer_ownership_many(zfs_refcount_t *, uint64_t,
void *, void *);
boolean_t zfs_refcount_held(zfs_refcount_t *, void *); boolean_t zfs_refcount_held(zfs_refcount_t *, void *);
boolean_t zfs_refcount_not_held(zfs_refcount_t *, void *); boolean_t zfs_refcount_not_held(zfs_refcount_t *, void *);
@ -106,8 +108,9 @@ typedef struct refcount {
atomic_add_64(&(src)->rc_count, -__tmp); \ atomic_add_64(&(src)->rc_count, -__tmp); \
atomic_add_64(&(dst)->rc_count, __tmp); \ atomic_add_64(&(dst)->rc_count, __tmp); \
} }
#define zfs_refcount_transfer_ownership(rc, current_holder, new_holder) (void)0 #define zfs_refcount_transfer_ownership(rc, ch, nh) ((void)0)
#define zfs_refcount_held(rc, holder) ((rc)->rc_count > 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) #define zfs_refcount_not_held(rc, holder) (B_TRUE)
#define zfs_refcount_init() #define zfs_refcount_init()

View File

@ -3081,8 +3081,8 @@ arc_share_buf(arc_buf_hdr_t *hdr, arc_buf_t *buf)
* refcount ownership to the hdr since it always owns * refcount ownership to the hdr since it always owns
* the refcount whenever an arc_buf_t is shared. * the refcount whenever an arc_buf_t is shared.
*/ */
zfs_refcount_transfer_ownership(&hdr->b_l1hdr.b_state->arcs_size, zfs_refcount_transfer_ownership_many(&hdr->b_l1hdr.b_state->arcs_size,
buf, hdr); arc_hdr_size(hdr), buf, hdr);
hdr->b_l1hdr.b_pabd = abd_get_from_buf(buf->b_data, arc_buf_size(buf)); 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, abd_take_ownership_of_buf(hdr->b_l1hdr.b_pabd,
HDR_ISTYPE_METADATA(hdr)); HDR_ISTYPE_METADATA(hdr));
@ -3110,8 +3110,8 @@ arc_unshare_buf(arc_buf_hdr_t *hdr, arc_buf_t *buf)
* We are no longer sharing this buffer so we need * We are no longer sharing this buffer so we need
* to transfer its ownership to the rightful owner. * to transfer its ownership to the rightful owner.
*/ */
zfs_refcount_transfer_ownership(&hdr->b_l1hdr.b_state->arcs_size, zfs_refcount_transfer_ownership_many(&hdr->b_l1hdr.b_state->arcs_size,
hdr, buf); arc_hdr_size(hdr), hdr, buf);
arc_hdr_clear_flags(hdr, ARC_FLAG_SHARED_DATA); arc_hdr_clear_flags(hdr, ARC_FLAG_SHARED_DATA);
abd_release_ownership_of_buf(hdr->b_l1hdr.b_pabd); abd_release_ownership_of_buf(hdr->b_l1hdr.b_pabd);
abd_put(hdr->b_l1hdr.b_pabd); abd_put(hdr->b_l1hdr.b_pabd);

View File

@ -234,8 +234,8 @@ zfs_refcount_transfer(zfs_refcount_t *dst, zfs_refcount_t *src)
} }
void void
zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder, zfs_refcount_transfer_ownership_many(zfs_refcount_t *rc, uint64_t number,
void *new_holder) void *current_holder, void *new_holder)
{ {
reference_t *ref; reference_t *ref;
boolean_t found = B_FALSE; boolean_t found = B_FALSE;
@ -248,7 +248,8 @@ zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder,
for (ref = list_head(&rc->rc_list); ref; for (ref = list_head(&rc->rc_list); ref;
ref = list_next(&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; ref->ref_holder = new_holder;
found = B_TRUE; found = B_TRUE;
break; break;
@ -258,6 +259,14 @@ zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder,
mutex_exit(&rc->rc_mtx); mutex_exit(&rc->rc_mtx);
} }
void
zfs_refcount_transfer_ownership(zfs_refcount_t *rc, void *current_holder,
void *new_holder)
{
return (zfs_refcount_transfer_ownership_many(rc, 1, current_holder,
new_holder));
}
/* /*
* If tracking is enabled, return true if a reference exists that matches * 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 * the "holder" tag. If tracking is disabled, then return true if a reference