Re: [PATCH RFC 00/11] lock monitor: Separate features related to lock

[Frederic Weisbecker <fweisbec@gmail.com>]

On Sun, Mar 14, 2010 at 07:13:55PM +0100, Peter Zijlstra wrote:
> On Sun, 2010-03-14 at 19:38 +0900, Hitoshi Mitake wrote:
> > Current lockdep is too complicated because,
> >  * dependency validation
> >  * statistics
> >  * event tracing
> > are all implemented by it.
> > This cause problem of overhead.
> > If user enables one of them, overhead of rests part is not avoidable.
> > (tracing is exception. If user enables validation or stat,
> > overhead of tracing doesn't occur.)
> > 
> > So I suggest new subsystem "lock monitor".
> > This is a general purpose lock event hooking mechanism.
> > 
> > lock monitor will be enable easy implementing and running
> > these features related to lock.
> > 
> > And I'm hoping that lock monitor will reduce overhead of perf lock.
> > Because lock monitor separates dependency validation and event tracing clearly,
> > so calling of functions of lockdep (e.g. lock_acquire()) only for validation
> > will not occur lock events.
> > 
> > I implemented it on the branch perf/inject of Frederic's random-tracing tree.
> > Because the branch is hottest place of lock and tracing :)
> 
> OK, so I really don't like this much..
> 
> Building a lockstat kernel (PROVE_LOCKING=n) should not have much more
> overhead than the proposed solution, if the simple lock acquistion
> tracking bothers you, you can do a patch to weaken that.
> 
> I really really dislike how you add a monitor variable between
> everything for no reason what so ever.
> 
> You use a new rwlock_t, which is an instant fail, those things are worse
> than useless.
> 
> You add chained indirect calls into all lock ops, that's got to hurt.


Well, the idea was not bad at the first glance. It was separating
lockdep and lock events codes.

But indeed, the indirect calls plus the locking are not good
for such a fast path.

There is something else, it would be nice to keep the
lockdep_map -> lockdep_class mapping so that we can
do lock profiling based on classes too. So we actually
need the lockdep code. What we don't need is the prove
locking or the lock stats. So I guess we can have a new
config to enable lock events and get rid of the prove
locking / lock stat code if we don't need it.