INVARIANTS vs DIAGNOSTIC % lf_advlockasync

[Konstantin Belousov]

On Wed, Dec 28, 2016 at 11:28:54AM +0200, Andriy Gapon wrote:
> 
> I wonder if there are any guidelines on when to use INVARIANTS vs DIAGNOSTIC vs
> something else.  Should the amount of output be taken into account?  Or the
> performance impact?  Or just the common sense?
> 
> What I really mean is that if some sanity check could be rather expensive (e.g.
> it needs to iterate over a potentially long list), what option should be used to
> enabled it?
> 
> I ask this question with one particular case in mind.
> lf_advlockasync() has a block of code under INVARIANTS with a loop over a list
> that has a nested loop over the same list for pair-wise checks.
> What's worse is that that code is executed with a lock held and that lock can
> potentially be highly contended (ls_lock).
> In our test environment we can observe the lock being held for as much as 125
> milliseconds resulting in a huge backlog on the lock.  (Even though the
> requested advisory locks are all shared locks and unlocks.)
There are at least two blocks of code in kern_lockf.c that you might want
to conditionally enable, one is lf_advlockasync(), another in the
beginning of lf_add_edge().

> 
> So, we would like to reduce the performance hit in that code, but still have the
> benefits of INVARIANTS enabled over all.
> 

We have a precedent with DIAGNOSTIC enabling very heavy weight checks,
see kern/subr_vmem.c enable_vmem_check sysctl.