From 5ba4025a8d94699d2638938a0cdf790113ff0531 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Mon, 5 Jun 2023 14:51:44 -0400 Subject: [PATCH] Introduce zfs_refcount_(add|remove)_few(). There are two places where we need to add/remove several references with semantics of zfs_refcount_(add|remove). But when debug/tracing is disabled, it is a crime to run multiple atomic_inc() in a loop, especially under congested pool-wide allocator lock. Introduced new functions implement the same semantics as the loop, but without overhead in production builds. Reviewed-by: Rich Ercolani Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #14934 --- include/sys/zfs_refcount.h | 12 +++++++++--- module/zfs/dmu_zfetch.c | 3 +-- module/zfs/metaslab.c | 6 ++---- module/zfs/refcount.c | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/sys/zfs_refcount.h b/include/sys/zfs_refcount.h index 42f846b89..4efa266a5 100644 --- a/include/sys/zfs_refcount.h +++ b/include/sys/zfs_refcount.h @@ -73,13 +73,15 @@ int64_t zfs_refcount_count(zfs_refcount_t *); int64_t zfs_refcount_add(zfs_refcount_t *, const void *); int64_t zfs_refcount_remove(zfs_refcount_t *, const void *); /* - * Note that (add|remove)_many add/remove one reference with "number" N, - * _not_ make N references with "number" 1, which is what vanilla - * zfs_refcount_(add|remove) would do if called N times. + * Note that (add|remove)_many adds/removes one reference with "number" N, + * _not_ N references with "number" 1, which is what (add|remove)_few does, + * or what vanilla zfs_refcount_(add|remove) called N times would do. * * Attempting to remove a reference with number N when none exists is a * panic on debug kernels with reference_tracking enabled. */ +void zfs_refcount_add_few(zfs_refcount_t *, uint64_t, const void *); +void zfs_refcount_remove_few(zfs_refcount_t *, uint64_t, const void *); int64_t zfs_refcount_add_many(zfs_refcount_t *, uint64_t, const void *); int64_t zfs_refcount_remove_many(zfs_refcount_t *, uint64_t, const void *); void zfs_refcount_transfer(zfs_refcount_t *, zfs_refcount_t *); @@ -108,6 +110,10 @@ typedef struct refcount { #define zfs_refcount_count(rc) atomic_load_64(&(rc)->rc_count) #define zfs_refcount_add(rc, holder) atomic_inc_64_nv(&(rc)->rc_count) #define zfs_refcount_remove(rc, holder) atomic_dec_64_nv(&(rc)->rc_count) +#define zfs_refcount_add_few(rc, number, holder) \ + atomic_add_64(&(rc)->rc_count, number) +#define zfs_refcount_remove_few(rc, number, holder) \ + atomic_add_64(&(rc)->rc_count, -number) #define zfs_refcount_add_many(rc, number, holder) \ atomic_add_64_nv(&(rc)->rc_count, number) #define zfs_refcount_remove_many(rc, number, holder) \ diff --git a/module/zfs/dmu_zfetch.c b/module/zfs/dmu_zfetch.c index ffc012e6c..b70459380 100644 --- a/module/zfs/dmu_zfetch.c +++ b/module/zfs/dmu_zfetch.c @@ -520,8 +520,7 @@ dmu_zfetch_run(zstream_t *zs, boolean_t missed, boolean_t have_lock) issued = pf_end - pf_start + ipf_end - ipf_start; if (issued > 1) { /* More references on top of taken in dmu_zfetch_prepare(). */ - for (int i = 0; i < issued - 1; i++) - zfs_refcount_add(&zs->zs_refs, NULL); + zfs_refcount_add_few(&zs->zs_refs, issued - 1, NULL); } else if (issued == 0) { /* Some other thread has done our work, so drop the ref. */ if (zfs_refcount_remove(&zs->zs_refs, NULL) == 0) diff --git a/module/zfs/metaslab.c b/module/zfs/metaslab.c index 94b131fcd..176247d63 100644 --- a/module/zfs/metaslab.c +++ b/module/zfs/metaslab.c @@ -5650,8 +5650,7 @@ metaslab_class_throttle_reserve(metaslab_class_t *mc, int slots, int allocator, * We reserve the slots individually so that we can unreserve * them individually when an I/O completes. */ - for (int d = 0; d < slots; d++) - zfs_refcount_add(&mca->mca_alloc_slots, zio); + zfs_refcount_add_few(&mca->mca_alloc_slots, slots, zio); zio->io_flags |= ZIO_FLAG_IO_ALLOCATING; return (B_TRUE); } @@ -5665,8 +5664,7 @@ metaslab_class_throttle_unreserve(metaslab_class_t *mc, int slots, metaslab_class_allocator_t *mca = &mc->mc_allocator[allocator]; ASSERT(mc->mc_alloc_throttle_enabled); - for (int d = 0; d < slots; d++) - zfs_refcount_remove(&mca->mca_alloc_slots, zio); + zfs_refcount_remove_few(&mca->mca_alloc_slots, slots, zio); } static int diff --git a/module/zfs/refcount.c b/module/zfs/refcount.c index 62ec03e10..c9a504f67 100644 --- a/module/zfs/refcount.c +++ b/module/zfs/refcount.c @@ -151,6 +151,15 @@ zfs_refcount_add(zfs_refcount_t *rc, const void *holder) return (zfs_refcount_add_many(rc, 1, holder)); } +void +zfs_refcount_add_few(zfs_refcount_t *rc, uint64_t number, const void *holder) +{ + if (!rc->rc_tracked) + (void) zfs_refcount_add_many(rc, number, holder); + else for (; number > 0; number--) + (void) zfs_refcount_add(rc, holder); +} + int64_t zfs_refcount_remove_many(zfs_refcount_t *rc, uint64_t number, const void *holder) @@ -204,6 +213,15 @@ zfs_refcount_remove(zfs_refcount_t *rc, const void *holder) return (zfs_refcount_remove_many(rc, 1, holder)); } +void +zfs_refcount_remove_few(zfs_refcount_t *rc, uint64_t number, const void *holder) +{ + if (!rc->rc_tracked) + (void) zfs_refcount_remove_many(rc, number, holder); + else for (; number > 0; number--) + (void) zfs_refcount_remove(rc, holder); +} + void zfs_refcount_transfer(zfs_refcount_t *dst, zfs_refcount_t *src) {