Re: [PATCH] tracing/core: Add current context on tracing recursion warning

["Frédéric Weisbecker" <fweisbec@gmail.com>]

2009/4/19 Ingo Molnar <mingo@elte.hu>:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> On Sun, Apr 19, 2009 at 04:01:54PM +0200, Ingo Molnar wrote:
>> > It would be nice to have this ... but there's no need to do that
>> > atomic thing - just use printk_once() please. (if we race with
>> > another instance and get two messages that's not a problem)
>> >
>> >     Ingo
>>
>>
>> Ah, indeed I forgot about printk_once()
>> I've updated the repo with the following v2 on:
>> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing tracing/recursion
>>
>> ---
>> >From e894732989e345ea012de146c37d71dd7ed3575d Mon Sep 17 00:00:00 2001
>> From: Frederic Weisbecker <fweisbec@gmail.com>
>> Date: Sun, 19 Apr 2009 16:18:16 +0200
>> Subject: [PATCH v2] tracing/core: Add current context on tracing recursion warning
>
>>
>> In case of tracing recursion detection, we only get the stacktrace.
>> But the current context may be very useful to debug the issue.
>>
>> This patch adds the softirq/hardirq/nmi context with the warning
>> using lockdep context display to have a familiar output.
>>
>> v2: Use printk_once()
>>
>> [ Impact: more information in tracing recursion ]
>>
>> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
>> ---
>>  kernel/trace/ring_buffer.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
>> index b421b0e..e315178 100644
>> --- a/kernel/trace/ring_buffer.c
>> +++ b/kernel/trace/ring_buffer.c
>> @@ -1495,6 +1495,16 @@ static int trace_recursive_lock(void)
>>       if (unlikely(current->trace_recursion & (1 << level))) {
>>               /* Disable all tracing before we do anything else */
>>               tracing_off_permanent();
>> +
>> +             printk_once(KERN_WARNING "Tracing recursion: "
>> +                         "[HC%u[%lu]:SC%u[%lu]:NMI[%lu]:HE%u:SE%u]\n",
>> +                         current->hardirq_context,
>> +                         hardirq_count() >> HARDIRQ_SHIFT,
>> +                         current->softirq_context,
>> +                         softirq_count() >> SOFTIRQ_SHIFT,
>> +                         in_nmi(), current->hardirqs_enabled,
>> +                         current->softirqs_enabled);
>> +
>>               WARN_ON_ONCE(1);
>
> Pulled, thanks Frederic!
>
> btw., we might have done it via:
>
>        if (WARN_ON_ONCE(1)) {
>                printk(...);
>        }
>
> as well ... but i have not checked how WARN_ON_ONCE() return value
> behaves after the first warning - does it return true or false?



I've checked it because I wanted to do that first :-)
It returns always 1 if the condition is true, not only once.

Btw, I've found the origin of the warning.
We drop the recursion protection on commit but not on discard.

Preparing a patch.


>        Ingo
>