Re: CVS commit: src/sys

[Greg Troxel <gdt%lexort.com@localhost>]

Kamil Rytarowski <n54%gmx.com@localhost> writes:

> On 08.05.2019 11:34, Ryota Ozaki wrote:
>> On Sat, Apr 20, 2019 at 6:45 PM Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>>>
>>> On Fri, Apr 19, 2019 at 6:49 PM Kamil Rytarowski <n54%gmx.com@localhost> wrote:
>>>>
>>>> On 19.04.2019 11:41, J. Hannken-Illjes wrote:
>>>>>> On 19. Apr 2019, at 03:52, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>>>>>>
>>>>>> Module Name: src
>>>>>> Committed By:        ozaki-r
>>>>>> Date:                Fri Apr 19 01:52:56 UTC 2019
>>>>>>
>>>>>> Modified Files:
>>>>>>      src/sys/kern: kern_lwp.c kern_softint.c subr_psref.c
>>>>>>      src/sys/rump/kern/lib/libsysproxy: sysproxy.c
>>>>>>      src/sys/sys: lwp.h userret.h
>>>>>>
>>>>>> Log Message:
>>>>>> Implement a simple psref leak detector
>>>>>>
>>>>>> It detects leaks by counting up the number of held psref by an LWP and checking
>>>>>> its zeroness at the end of syscalls and softint handlers.  For the counter, a
>>>>>> unused field of struct lwp is reused.
>>>>>
>>>>> For DIAGNOSTIC-only operations LWP specific data
>>>>> (see kern/subr_lwp_specificdata.c) is a better choice.
>>>>>
>>>>
>>>> I wanted to propose the same. An exampe of this is in KCOV.
>>>
>>> Thanks.  I'll try it.  (I'll be AFK for the next few days...)
>> 
>> I'm sorry for delaying this task.  Finally I have benchmarked a revised patch
>> (our benchmarking setups have been out of service for a couple of weeks...).
>> 
>> Performance degradation of IP forwarding is 3%.  Is it acceptable as
>> DIAGNOSTIC?
>> 
>
> For DIAGNOSTIC should be fine.

I think 3% is too much for DIAGNOSTIC.   DEBUG, sure, and a specific
option to turn it on seems fine.  DIAGNOSTIC historically has been only
for things that check invariants and assert, such that if you don't mind
the crashes when detecting things, there is no performance reason to
avoid it.