Patchwork [1/1] Ubuntu: Sauce: Disable function tracing after hitting __schedule_bug

login
register
mail settings
Submitter Chase Douglas
Date March 12, 2010, 2:23 p.m.
Message ID <1268403820-13556-2-git-send-email-chase.douglas@canonical.com>
Download mbox | patch
Permalink /patch/47680/
State Accepted
Delegated to: Andy Whitcroft
Headers show

Comments

Chase Douglas - March 12, 2010, 2:23 p.m.
If we hit __schedule_bug, then something bad has happened, and it would
be helpful to get an accurate function trace to see what's going wrong.
The debugfs function tracer works great for this task, but it spits out
a ton of information both before and after the real bug occurs. This
change stops the tracing when we enter __schedule_bug, so the tracing
report is smaller and leaves the developer with the needed information
right at the end of the trace.

This should not cause a regression or any negative effects unless
someone truly wanted to trace through a __schedule_bug call. In that
case, they can recompile their own kernel without this call. The vast
majority of tracings that hit __schedule_bug would be enhanced by this
change.

To see how tracing with this change can be used for "scheduling while
atomic" bugs, see http://bugs.launchpad.net/bugs/534549.

Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
---
 kernel/sched.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Tim Gardner - March 12, 2010, 3:09 p.m.
On 03/12/2010 07:23 AM, Chase Douglas wrote:
> If we hit __schedule_bug, then something bad has happened, and it would
> be helpful to get an accurate function trace to see what's going wrong.
> The debugfs function tracer works great for this task, but it spits out
> a ton of information both before and after the real bug occurs. This
> change stops the tracing when we enter __schedule_bug, so the tracing
> report is smaller and leaves the developer with the needed information
> right at the end of the trace.
>
> This should not cause a regression or any negative effects unless
> someone truly wanted to trace through a __schedule_bug call. In that
> case, they can recompile their own kernel without this call. The vast
> majority of tracings that hit __schedule_bug would be enhanced by this
> change.
>
> To see how tracing with this change can be used for "scheduling while
> atomic" bugs, see http://bugs.launchpad.net/bugs/534549.
>
> Signed-off-by: Chase Douglas<chase.douglas@canonical.com>
> ---
>   kernel/sched.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 4d01095..0d2ba50 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -5341,6 +5341,8 @@ static noinline void __schedule_bug(struct task_struct *prev)
>   {
>   	struct pt_regs *regs = get_irq_regs();
>
> +	tracing_off();
> +
>   	printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n",
>   		prev->comm, prev->pid, preempt_count());
>

Seems reasonable enough. tracing_off() is dead simple, so ought not 
cause a regression. Have you discussed this with upstream as to why this 
isn't the default behavior?

Acked-by: Tim Gardner <tim.gardner@canonical.com>
Chase Douglas - March 12, 2010, 3:23 p.m.
On Fri, Mar 12, 2010 at 10:09 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
> On 03/12/2010 07:23 AM, Chase Douglas wrote:
>>
>> If we hit __schedule_bug, then something bad has happened, and it would
>> be helpful to get an accurate function trace to see what's going wrong.
>> The debugfs function tracer works great for this task, but it spits out
>> a ton of information both before and after the real bug occurs. This
>> change stops the tracing when we enter __schedule_bug, so the tracing
>> report is smaller and leaves the developer with the needed information
>> right at the end of the trace.
>>
>> This should not cause a regression or any negative effects unless
>> someone truly wanted to trace through a __schedule_bug call. In that
>> case, they can recompile their own kernel without this call. The vast
>> majority of tracings that hit __schedule_bug would be enhanced by this
>> change.
>>
>> To see how tracing with this change can be used for "scheduling while
>> atomic" bugs, see http://bugs.launchpad.net/bugs/534549.
>>
>> Signed-off-by: Chase Douglas<chase.douglas@canonical.com>
>> ---
>>  kernel/sched.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/sched.c b/kernel/sched.c
>> index 4d01095..0d2ba50 100644
>> --- a/kernel/sched.c
>> +++ b/kernel/sched.c
>> @@ -5341,6 +5341,8 @@ static noinline void __schedule_bug(struct
>> task_struct *prev)
>>  {
>>        struct pt_regs *regs = get_irq_regs();
>>
>> +       tracing_off();
>> +
>>        printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n",
>>                prev->comm, prev->pid, preempt_count());
>>
>
> Seems reasonable enough. tracing_off() is dead simple, so ought not cause a
> regression. Have you discussed this with upstream as to why this isn't the
> default behavior?

I haven't, but I'll send a note out to linux-kernel. I'm guessing the
reason is that tracing can be used for many different reasons, and
they don't want stray tracing_off calls to kill legitimate tracing in
a vanilla kernel. In our case, using tracing_off in __schedule_bug
will help us solve scheduling while atomic bugs faster, and we aren't
terribly interested in academic uses of tracing in our official
kernels.
Andy Whitcroft - March 12, 2010, 5:15 p.m.
On Fri, Mar 12, 2010 at 10:23:45AM -0500, Chase Douglas wrote:

> >> +       tracing_off();
> >> +

How about adding a __read_mostly toggle for this set by a boot
parameter:

	if (likely(sched_tracing_stop))
		tracing_off();

-apw
Chase Douglas - March 12, 2010, 5:30 p.m.
On Fri, Mar 12, 2010 at 12:15 PM, Andy Whitcroft <apw@canonical.com> wrote:
> On Fri, Mar 12, 2010 at 10:23:45AM -0500, Chase Douglas wrote:
>
>> >> +       tracing_off();
>> >> +
>
> How about adding a __read_mostly toggle for this set by a boot
> parameter:
>
>        if (likely(sched_tracing_stop))
>                tracing_off();
>
> -apw

Possible, but a more invasive patch. If we think people will ever want
to disable it, then it's probably worth while. I'm just not sure
whether people will ever want to do so in an official Ubuntu kernel.
Andy Whitcroft - March 31, 2010, 10:39 a.m.
On Fri, Mar 12, 2010 at 09:23:40AM -0500, Chase Douglas wrote:
> If we hit __schedule_bug, then something bad has happened, and it would
> be helpful to get an accurate function trace to see what's going wrong.
> The debugfs function tracer works great for this task, but it spits out
> a ton of information both before and after the real bug occurs. This
> change stops the tracing when we enter __schedule_bug, so the tracing
> report is smaller and leaves the developer with the needed information
> right at the end of the trace.
> 
> This should not cause a regression or any negative effects unless
> someone truly wanted to trace through a __schedule_bug call. In that
> case, they can recompile their own kernel without this call. The vast
> majority of tracings that hit __schedule_bug would be enhanced by this
> change.
> 
> To see how tracing with this change can be used for "scheduling while
> atomic" bugs, see http://bugs.launchpad.net/bugs/534549.
> 
> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> ---
>  kernel/sched.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 4d01095..0d2ba50 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -5341,6 +5341,8 @@ static noinline void __schedule_bug(struct task_struct *prev)
>  {
>  	struct pt_regs *regs = get_irq_regs();
>  
> +	tracing_off();
> +
>  	printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n",
>  		prev->comm, prev->pid, preempt_count());
>  

Seems reasonable if it make debugging easier.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Chase Douglas - March 31, 2010, 12:14 p.m.
On Wed, Mar 31, 2010 at 6:39 AM, Andy Whitcroft <apw@canonical.com> wrote:
> On Fri, Mar 12, 2010 at 09:23:40AM -0500, Chase Douglas wrote:
>> If we hit __schedule_bug, then something bad has happened, and it would
>> be helpful to get an accurate function trace to see what's going wrong.
>> The debugfs function tracer works great for this task, but it spits out
>> a ton of information both before and after the real bug occurs. This
>> change stops the tracing when we enter __schedule_bug, so the tracing
>> report is smaller and leaves the developer with the needed information
>> right at the end of the trace.
>>
>> This should not cause a regression or any negative effects unless
>> someone truly wanted to trace through a __schedule_bug call. In that
>> case, they can recompile their own kernel without this call. The vast
>> majority of tracings that hit __schedule_bug would be enhanced by this
>> change.
>>
>> To see how tracing with this change can be used for "scheduling while
>> atomic" bugs, see http://bugs.launchpad.net/bugs/534549.
>>
>> Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
>> ---
>>  kernel/sched.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/sched.c b/kernel/sched.c
>> index 4d01095..0d2ba50 100644
>> --- a/kernel/sched.c
>> +++ b/kernel/sched.c
>> @@ -5341,6 +5341,8 @@ static noinline void __schedule_bug(struct task_struct *prev)
>>  {
>>       struct pt_regs *regs = get_irq_regs();
>>
>> +     tracing_off();
>> +
>>       printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n",
>>               prev->comm, prev->pid, preempt_count());
>>
>
> Seems reasonable if it make debugging easier.
>
> Acked-by: Andy Whitcroft <apw@canonical.com>

Just FYI, I have a better set of patches that do the same thing for
other areas like BUG() and WARN(), and it's enabled/disabled through a
boot option. However, it hasn't been accepted upstream yet. I think
it's best to just go with this for now since it's smaller and less
likely to cause a regression.

-- Chase

Patch

diff --git a/kernel/sched.c b/kernel/sched.c
index 4d01095..0d2ba50 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -5341,6 +5341,8 @@  static noinline void __schedule_bug(struct task_struct *prev)
 {
 	struct pt_regs *regs = get_irq_regs();
 
+	tracing_off();
+
 	printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n",
 		prev->comm, prev->pid, preempt_count());