svn commit: r285664 - in head/sys: kern sys

[Bruce Evans]

On Sat, 18 Jul 2015, Mark Johnston wrote:

> On Sat, Jul 18, 2015 at 08:55:07PM +1000, Bruce Evans wrote:
>> On Sat, 18 Jul 2015, Mark Johnston wrote:
>>
>>> Log:
>>>  Pass the lock object to lockstat_nsecs() and return immediately if
>>>  LO_NOPROFILE is set. Some timecounter handlers acquire a spin mutex, and
>>>  we don't want to recurse if lockstat probes are enabled.
>>
>> It is an error to call timecounter code from a low-level place like the
>> mutex implementation.  This workaround depends on all locks in timecounter
>> handlers being LO_NOPROFILE, and that breaks profiling of these locks.
>> ...
>
> I noticed that lock_profile (which predates lockstat a bit) does the
> exact same thing to avoid recursion. Specifically,
> lock_profile_obtain_lock_{success,failed} return immediately if
> LO_NOPROFILE is set on the target lock. As you pointed out,
> this change breaks profiling of timecounter locks, but the only
> timecounter implementation in the tree that currently acquires a lock
> during a timer read is i8254.

lock_profile also has another copy of lockstat_nsecs() (spelled
nanoseconds()), with different style bugs.  The style bugs start with
lockstat_nsecs()'s existence and nanoseconds()'s name being too generic.

> The other two locks that set MTX_NOPROFILE are the witness lock and the
> i386 icu lock. Lock order checking can usually be done without taking
> the witness lock, so contention would be unusual, and it would strike
> me as strange to profile locking with witness enabled anyway. :)
> I'm not sure why i386's icu_lock has profiling disabled; this was done
> in r166001, but the commit log doesn't explain the reason.

I didn't know that lock profiling was independent of witness.

I can't see any reason why profiling must be disabled for icu_lock, but
perhaps it should be disabled for efficiency reasons for all low-level
mutexes.

Low-level mutexes now use combinations of MTX_QUIET and MTX_NOWITNESS
with no apparent pattern.  It seems right to log everything and check
everything by default (except witness's lock must not witness itself
recursively), and never hard-code hiding from witness or anything just
for efficiency.  Then if a locking error is found in a console driver
lock (there are many such errors that are not found now), the error
must not be reported on the consoles with the locking error.
MTX_QUIET's name suggests that it controls printing of error messages,
but its documentation is ambiguous: it controls "logging" and it isn't
clear if that is in-memory (for future use by witness or anything) or
just printing.

Bruce