Re: [PATCH] [RESEND v2] tracing/ftrace: Introduce the big kernel lock tracer

[Ingo Molnar <mingo@elte.hu>]

* Frédéric Weisbecker <fweisbec@gmail.com> wrote:

> 2008/10/27 Ingo Molnar <mingo@elte.hu>:
> >
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >
> >> diff --git a/lib/kernel_lock.c b/lib/kernel_lock.c
> >> index 01a3c22..45828b2 100644
> >> --- a/lib/kernel_lock.c
> >> +++ b/lib/kernel_lock.c
> >> @@ -9,6 +9,7 @@
> >>  #include <linux/module.h>
> >>  #include <linux/kallsyms.h>
> >>  #include <linux/semaphore.h>
> >> +#include <trace/bkl.h>
> >>
> >>  /*
> >>   * The 'big kernel lock'
> >> @@ -107,6 +108,37 @@ static inline void __unlock_kernel(void)
> >>       preempt_enable();
> >>  }
> >>
> >> +#ifdef CONFIG_BKL_TRACER
> >> +static void lock_kernel_trace(void)
> >> +{
> >> +     int cpu;
> >> +     struct bkl_trace_acquire trace;
> >> +
> >> +     preempt_disable();
> >> +     cpu = raw_smp_processor_id();
> >> +     preempt_enable();
> >> +
> >> +     trace.acquire_req_time = cpu_clock(cpu);
> >> +     __lock_kernel();
> >> +     trace.acquire_time = cpu_clock(cpu);
> >> +     trace_bkl_acquire(&trace);
> >> +}
> >> +
> >> +static void unlock_kernel_trace(void)
> >> +{
> >> +     struct bkl_trace_release trace;
> >> +     trace.release_time = cpu_clock(raw_smp_processor_id());
> >> +     trace_bkl_release(&trace);
> >> +     __unlock_kernel();
> >> +}
> >> +
> >> +#else
> >> +
> >> +#define lock_kernel_trace() __lock_kernel()
> >> +#define unlock_kernel_trace() __unlock_kernel()
> >> +
> >> +#endif
> >
> > hm, this looks a bit ugly.
> >
> > are you aware of the tip/kill-the-BKL branch? It's an old-ish but
> > otherwise sane branch that needs some refreshing (hence it's not part
> > of tip/master).
> >
> > Once we have that "kill the BKL by turning it into a mutex" feature
> > alive, and have fixed the places that rely on odd properties of the
> > BKL, the BKL becomes just an ordinary mutex and we could trace its
> > latencies via the existing lockdep/lockstat callbacks.
> >
> > and we could trace all the other mutexes as well.
> 
> 
> No problem, we can forget about it. My goal was to produce some 
> statistics to locate the points that most often hold the bkl. That 
> would help to define some priorities on which bkl holding is to 
> remove first.
> 
> But if that would be better to rather invest the time on the 
> kill-the-bkl tree (which I thought was dead), so I would be pleased 
> to help.

the kill-the-BKL tree is not dead, just inactive. Looking for a brave 
volunteer to merge it up to latest, to boot it with 
CONFIG_PROVE_LOCKING=y and to have a good look at all the BKL locking 
output that lockdep might disable.

	Ingo