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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
