> Date: Mon, 7 Aug 2017 13:08:35 +0900 > From: Ryota Ozaki <ozak...@netbsd.org> > > Anyway I've brought another (stupid) approach that uses an atomic > counter: > http://www.netbsd.org/~ozaki-r/localcount_debug_atomic.diff > > It counts up a counter with atomic instructions along with localcount > if LOCKDEBUG enabled. Of course it blows the merit of localcount but > I can provide an accurate view of the counter. > > Do you think that it works and is useful?
That looks plausible. I'd put it under DEBUG && LOCKDEBUG, and require localcount_debug_refcnt to be used only in KDASSERT, to avoid adding yet more contention to plain LOCKDEBUG, and to make it safe to write, e.g., KDASSERT(localcount_debug_refcnt(lc) >= 2). Otherwise, under, e.g., DIAGNOSTIC && !LOCKDEBUG, KASSERT(localcount_debug_refcnt(lc) >= 2) would always fail, since localcount_debug_refcnt always returns 0. Tiny nit: I suggest you always use uint32_t for this, and use atomic_dec_32_nv(&lc->lc_refcnt) == UINT_MAX to detect underflow, rather than conversion to int32_t. Maybe check-and-panic on overflow too: if atomic_inc_32_nv(&lc->lc_refcnt) == 0, panic. Problem: The ABI is different with and without LOCKDEBUG. The structure member can't be conditional. I don't see a problem with changing always adding another 32-bit (or even 64-bit) member to struct localcount -- they already require O(nobj * ncpu) memory, so an extra handful of bytes per object is inconsequential -- so I'd just make it unconditional. No need for #include <sys/atomic.h> in localcount.h. lc_refcnt member should be volatile-qualified.