From 84d0c4d7eca5a558115f6417889459d676c3ebec Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Sat, 28 Oct 2023 05:36:15 +0000 Subject: [PATCH 1/3] Add atomic_dec_not_last_64 Signed-off-by: Mateusz Guzik --- include/os/freebsd/spl/sys/atomic.h | 17 +++++++++++++++++ include/os/linux/spl/sys/atomic.h | 1 + lib/libspl/atomic.c | 18 ++++++++++++++++++ lib/libspl/include/atomic.h | 1 + 4 files changed, 37 insertions(+) diff --git a/include/os/freebsd/spl/sys/atomic.h b/include/os/freebsd/spl/sys/atomic.h index 40a67704fd..2dbb372ca2 100644 --- a/include/os/freebsd/spl/sys/atomic.h +++ b/include/os/freebsd/spl/sys/atomic.h @@ -149,6 +149,23 @@ atomic_cas_64(volatile uint64_t *target, uint64_t cmp, uint64_t newval) #endif #endif +static inline int +atomic_dec_not_last_64(volatile uint64_t *target) +{ + uint64_t val, oldval; + + val = *target; + for (;;) { + if (val <= 1) { + return (0); + } + oldval = atomic_cas_64(target, val, val - 1); + if (oldval == val) + return (1); + val = oldval; + } +} + static __inline void atomic_inc_64(volatile uint64_t *target) { diff --git a/include/os/linux/spl/sys/atomic.h b/include/os/linux/spl/sys/atomic.h index 8f7fa5aeda..141ea94841 100644 --- a/include/os/linux/spl/sys/atomic.h +++ b/include/os/linux/spl/sys/atomic.h @@ -62,6 +62,7 @@ #define atomic_swap_64(v, x) atomic64_xchg((atomic64_t *)(v), x) #define atomic_load_64(v) atomic64_read((atomic64_t *)(v)) #define atomic_store_64(v, x) atomic64_set((atomic64_t *)(v), x) +#define atomic_dec_not_last_64(v) atomic64_dec_if_positive((atomic64_t *)(v)) #ifdef _LP64 static __inline__ void * diff --git a/lib/libspl/atomic.c b/lib/libspl/atomic.c index 8cc350710b..a37feb25a2 100644 --- a/lib/libspl/atomic.c +++ b/lib/libspl/atomic.c @@ -62,6 +62,24 @@ ATOMIC_DEC(uchar, uchar_t) ATOMIC_DEC(ushort, ushort_t) ATOMIC_DEC(uint, uint_t) ATOMIC_DEC(ulong, ulong_t) + +int +atomic_dec_not_last_64(volatile uint64_t *target) +{ + uint64_t val, oldval; + + val = *target; + for (;;) { + if (val <= 1) { + return (0); + } + oldval = atomic_cas_64(target, val, val - 1); + if (oldval == val) + return (1); + val = oldval; + } +} + /* END CSTYLED */ diff --git a/lib/libspl/include/atomic.h b/lib/libspl/include/atomic.h index 4ebdbbda98..c234cecadc 100644 --- a/lib/libspl/include/atomic.h +++ b/lib/libspl/include/atomic.h @@ -155,6 +155,7 @@ extern uint_t atomic_dec_uint_nv(volatile uint_t *); extern ulong_t atomic_dec_ulong_nv(volatile ulong_t *); #if defined(_INT64_TYPE) extern uint64_t atomic_dec_64_nv(volatile uint64_t *); +extern int atomic_dec_not_last_64(volatile uint64_t *); #endif /* From eee1810861ba01690366c746a58f300b49a1a321 Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Sat, 28 Oct 2023 05:36:57 +0000 Subject: [PATCH 2/3] Add zfs_refcount_remove_not_last Signed-off-by: Mateusz Guzik --- include/sys/zfs_refcount.h | 4 ++++ module/zfs/refcount.c | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/include/sys/zfs_refcount.h b/include/sys/zfs_refcount.h index 77965a0aa5..5a1bd400f5 100644 --- a/include/sys/zfs_refcount.h +++ b/include/sys/zfs_refcount.h @@ -76,6 +76,8 @@ int zfs_refcount_is_zero(zfs_refcount_t *); 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 *); +int zfs_refcount_remove_not_last(zfs_refcount_t *, const void *); + /* * 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, @@ -114,6 +116,8 @@ 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_remove_not_last(rc, holder) \ + atomic_dec_not_last_64(&(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) \ diff --git a/module/zfs/refcount.c b/module/zfs/refcount.c index 718bbb34a8..bee8f6f231 100644 --- a/module/zfs/refcount.c +++ b/module/zfs/refcount.c @@ -221,6 +221,47 @@ zfs_refcount_remove_few(zfs_refcount_t *rc, uint64_t number, const void *holder) (void) zfs_refcount_remove(rc, holder); } +int +zfs_refcount_remove_not_last(zfs_refcount_t *rc, const void *holder) +{ + reference_t *ref, s; + + if (likely(!rc->rc_tracked)) { + return (atomic_dec_not_last_64(&(rc)->rc_count)); + } + + s.ref_holder = holder; + s.ref_number = 1; + s.ref_search = B_TRUE; + mutex_enter(&rc->rc_mtx); + ASSERT3U(rc->rc_count, >=, 1); + ref = avl_find(&rc->rc_tree, &s, NULL); + if (unlikely(ref == NULL)) { + panic("No such hold %p on refcount %llx", holder, + (u_longlong_t)(uintptr_t)rc); + return (0); + } + if (rc->rc_count == 1) { + mutex_exit(&rc->rc_mtx); + return (0); + } + avl_remove(&rc->rc_tree, ref); + if (reference_history > 0) { + list_insert_head(&rc->rc_removed, ref); + if (rc->rc_removed_count >= reference_history) { + ref = list_remove_tail(&rc->rc_removed); + kmem_cache_free(reference_cache, ref); + } else { + rc->rc_removed_count++; + } + } else { + kmem_cache_free(reference_cache, ref); + } + rc->rc_count -= 1; + mutex_exit(&rc->rc_mtx); + return (1); +} + void zfs_refcount_transfer(zfs_refcount_t *dst, zfs_refcount_t *src) { From 7acd62b609e5afb107fe29bc4ceceab52679b6f7 Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Sat, 28 Oct 2023 05:37:24 +0000 Subject: [PATCH 3/3] Employ zfs_refcount_remove_unless_last in sa_idx_tab_rele This avoids taking the lock in the common case. This almost halves contention on z_hold_mtx as it avoids extending its hold time by not contending on sa_lock. Performance does not improve because contention shifts to other global-ish locks. Before: 108608922 (sx:zfsvfs->z_hold_mtx[i]) 15464416 (sleep mutex:vnode_list) 12455346 (sx:os->os_lock) 10792604 (sx:sa->sa_lock) 3888049 (sx:zfsvfs->z_znodes_lock) 2863341 (sleep mutex:struct mount mtx) 835414 (sx:dn->dn_mtx) After: 57594050 (sx:zfsvfs->z_hold_mtx[i]) 22089617 (sleep mutex:vnode_list) 12148265 (sx:zfsvfs->z_znodes_lock) 9537666 (sx:os->os_lock) 6833254 (sleep mutex:struct mount mtx) 1131875 (sx:sa->sa_lock) 437455 (sx:dn->dn_mtx) Signed-off-by: Mateusz Guzik --- module/zfs/sa.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/module/zfs/sa.c b/module/zfs/sa.c index f9daaabbed..edd50cd475 100644 --- a/module/zfs/sa.c +++ b/module/zfs/sa.c @@ -1325,6 +1325,9 @@ sa_idx_tab_rele(objset_t *os, void *arg) if (idx_tab == NULL) return; + if (zfs_refcount_remove_not_last(&idx_tab->sa_refcount, NULL)) + return; + mutex_enter(&sa->sa_lock); if (zfs_refcount_remove(&idx_tab->sa_refcount, NULL) == 0) { list_remove(&idx_tab->sa_layout->lot_idx_tab, idx_tab);