On Sat, May 11, 2019 at 10:49 AM Greg Troxel <g...@lexort.com> wrote: > > Kamil Rytarowski <n...@gmx.com> writes: > > > On 08.05.2019 11:34, Ryota Ozaki wrote: > >> On Sat, Apr 20, 2019 at 6:45 PM Ryota Ozaki <ozak...@netbsd.org> wrote: > >>> > >>> On Fri, Apr 19, 2019 at 6:49 PM Kamil Rytarowski <n...@gmx.com> wrote: > >>>> > >>>> On 19.04.2019 11:41, J. Hannken-Illjes wrote: > >>>>>> On 19. Apr 2019, at 03:52, Ryota Ozaki <ozak...@netbsd.org> 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.
So we have two options: 1. Keep the original (enabled on DIAGNOSTIC) - Its performance impact is negligible 2. Migrate to use lwp_specificadata and enable the feature on DEBUG (and something) I prefer to 1. because I want the feature to be enabled by many users (I assume that users (of -current) tend to enable DIAGNOSTIC and not DEBUG). If the detector is not enabled, a psref leak appears as a form that is difficult to know where the leak occurs; a fatal page fault occurs on psref_target_destroy that is a completely different context. If enabled, we can at least know a suspect LWP or softint handler and may find a cause in luck. ozaki-r