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

From: Ingo Molnar
Date: Mon Oct 27 2008 - 14:28:36 EST



* Frédéric Weisbecker <fweisbec@xxxxxxxxx> wrote:

> 2008/10/27 Ingo Molnar <mingo@xxxxxxx>:
> >
> > * Frederic Weisbecker <fweisbec@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/