Re: Debugging function of localcount

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Tue, Aug 8, 2017 at 3:16 AM, Taylor R Campbell
<campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
>> Date: Mon, 7 Aug 2017 13:08:35 +0900
>> From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>>
>> 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.

I'm sorry for not replying this thread. I'm back.

I've revised the patch as your suggestions:
  http://www.netbsd.org/~ozaki-r/localcount_debug_atomic.revised.diff

Thanks,
  ozaki-r