Re: [PATCH] lockdep: lock contention tracking

[Ingo Molnar <mingo@elte.hu>]

* Bill Huey <billh@gnuppy.monkey.org> wrote:

> On Mon, May 21, 2007 at 08:08:28AM +0200, Ingo Molnar wrote:
> > To me it appears Peter's stuff is already a pretty complete solution on 
> > its own, and it's a whole lot simpler (and less duplicative) than your 
> > lockstat patch. Could you list the specific items/features that you 
> > think Peter's stuff doesnt have?
> 
> First of all, this isn't an either/or kind of thing nor should it be 
> thought of in that way.

why? Please be specific.

> Precise file/function/line placement for one thing. [...]

Have you looked at the output Peter's patch produces? It prints out 
precise symbols:

 dcache_lock: 3000 0 [618] [<ffffffff8033badd>] _atomic_dec_and_lock+0x39/0x58

which can easily be turned into line numbers using debuginfo packages or 
using gdb. (But normally one only needs the symbol name, and we 
certainly do not want to burden the kernel source with tracking 
__FILE__/__LINE__ metadata, if the same is already available via 
CONFIG_DEBUG_INFO.)

anything else?

> [...] My stuff is potentially more extensible since I have other ideas 
> for it that really are outside of the lockdep logic currently. [...]

what do you mean, specifically?

> That's why I was hessitant to completely unify with lockdep in the 
> manner you suggested.

i really need specifics. Currently i have the choice between your stuff:

   17 files changed, 1425 insertions(+), 80 deletions(-)

and Peter's patch:

    6 files changed,  266 insertions(+), 18 deletions(-)

and Peter's patch (if it works out fine in testing - and it seemed fine 
so far on my testbox), is smaller, more maintainable, better integrated 
and thus the clear candidate for merging into -rt and merging upstream 
as well. It's far cleaner than i hoped this whole lock-stats thing could 
be done based on lockdep, so i'm pretty happy with Peter's current patch 
already.

	Ingo