Stop tracing on a schedule bug
in [Kernel]
|
From: Frederic Weisbecker on 15 Apr 2010 17:10 On Wed, Apr 14, 2010 at 12:20:16PM -0400, Chase Douglas wrote: > This change adds a tracing_off_event() call to stop tracing on schedule > bugs unless tracing_off=none was specified on the commandline. > > Signed-off-by: Chase Douglas <chase.douglas(a)canonical.com> > --- > kernel/sched.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/kernel/sched.c b/kernel/sched.c > index 6af210a..439f036 100644 > --- a/kernel/sched.c > +++ b/kernel/sched.c > @@ -3590,6 +3590,8 @@ static noinline void __schedule_bug(struct task_struct *prev) > { > struct pt_regs *regs = get_irq_regs(); > > + tracing_off_event(TRACE_EVENT_BUG); > + > printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n", > prev->comm, prev->pid, preempt_count()); I would rather call that a TRACE_EVENT_WARN as this is what happens: we warn but we continue. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Chase Douglas on 15 Apr 2010 17:50 On Thu, Apr 15, 2010 at 2:03 PM, Frederic Weisbecker <fweisbec(a)gmail.com> wrote: > On Wed, Apr 14, 2010 at 12:20:16PM -0400, Chase Douglas wrote: >> This change adds a tracing_off_event() call to stop tracing on schedule >> bugs unless tracing_off=none was specified on the commandline. >> >> Signed-off-by: Chase Douglas <chase.douglas(a)canonical.com> >> --- >> �kernel/sched.c | � �2 ++ >> �1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/kernel/sched.c b/kernel/sched.c >> index 6af210a..439f036 100644 >> --- a/kernel/sched.c >> +++ b/kernel/sched.c >> @@ -3590,6 +3590,8 @@ static noinline void __schedule_bug(struct task_struct *prev) >> �{ >> � � � struct pt_regs *regs = get_irq_regs(); >> >> + � � tracing_off_event(TRACE_EVENT_BUG); >> + >> � � � printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n", >> � � � � � � � prev->comm, prev->pid, preempt_count()); > > > > I would rather call that a TRACE_EVENT_WARN as this is what happens: we > warn but we continue. I tend to think of the TRACE_EVENT_* as an indication of severity and whether we want to stop the trace by default. From a distro standpoint, the likelihood that we want to continue tracing after a __schedule_bug is pretty low. It's easiest if we don't have to tell our users to add a kernel command line, especially since grub in Ubuntu 10.04 LTS is difficult to interact with for end users. Now I'm not much of an upstream developer (at least not yet), so if it makes much more sense to continue by default from a development standpoint, then perhaps it's better to make it continue by default. I just think that the frequency of hitting this should be low enough that it won't trip people up. And when it does, maybe that gives more incentive to fix the schedule bug :). -- Chase -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on 15 Apr 2010 19:10 On Thu, 15 Apr 2010, Chase Douglas wrote: > On Thu, Apr 15, 2010 at 2:03 PM, Frederic Weisbecker <fweisbec(a)gmail.com> wrote: > > On Wed, Apr 14, 2010 at 12:20:16PM -0400, Chase Douglas wrote: > >> This change adds a tracing_off_event() call to stop tracing on schedule > >> bugs unless tracing_off=none was specified on the commandline. > >> > >> Signed-off-by: Chase Douglas <chase.douglas(a)canonical.com> > >> --- > >> �kernel/sched.c | � �2 ++ > >> �1 files changed, 2 insertions(+), 0 deletions(-) > >> > >> diff --git a/kernel/sched.c b/kernel/sched.c > >> index 6af210a..439f036 100644 > >> --- a/kernel/sched.c > >> +++ b/kernel/sched.c > >> @@ -3590,6 +3590,8 @@ static noinline void __schedule_bug(struct task_struct *prev) > >> �{ > >> � � � struct pt_regs *regs = get_irq_regs(); > >> > >> + � � tracing_off_event(TRACE_EVENT_BUG); > >> + > >> � � � printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n", > >> � � � � � � � prev->comm, prev->pid, preempt_count()); > > > > > > > > I would rather call that a TRACE_EVENT_WARN as this is what happens: we > > warn but we continue. > > I tend to think of the TRACE_EVENT_* as an indication of severity and > whether we want to stop the trace by default. From a distro > standpoint, the likelihood that we want to continue tracing after a > __schedule_bug is pretty low. It's easiest if we don't have to tell Well, scheduling while atomic is a BUG, but one of the category which allows the kernel to continue. So in fact it's treated like a WARN_ON. So the tracing_off_event() qualifier should be *_WARN. That's independent of the question whether you want to stop tracing in that very case. Though I agree that the tracer should stop here. > our users to add a kernel command line, especially since grub in > Ubuntu 10.04 LTS is difficult to interact with for end users. That's a serious PITA caused by the "let's mimic the other OS" crowd and no excuse for creating a mess in the kernel. Thanks, tglx
From: Frederic Weisbecker on 15 Apr 2010 19:20 On Fri, Apr 16, 2010 at 01:01:50AM +0200, Thomas Gleixner wrote: > > I tend to think of the TRACE_EVENT_* as an indication of severity and > > whether we want to stop the trace by default. From a distro > > standpoint, the likelihood that we want to continue tracing after a > > __schedule_bug is pretty low. It's easiest if we don't have to tell > > Well, scheduling while atomic is a BUG, but one of the category which > allows the kernel to continue. So in fact it's treated like a WARN_ON. > So the tracing_off_event() qualifier should be *_WARN. > > That's independent of the question whether you want to stop tracing in > that very case Exactly. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Chase Douglas on 15 Apr 2010 19:30 On Thu, Apr 15, 2010 at 4:01 PM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > On Thu, 15 Apr 2010, Chase Douglas wrote: >> On Thu, Apr 15, 2010 at 2:03 PM, Frederic Weisbecker <fweisbec(a)gmail.com> wrote: >> > On Wed, Apr 14, 2010 at 12:20:16PM -0400, Chase Douglas wrote: >> >> This change adds a tracing_off_event() call to stop tracing on schedule >> >> bugs unless tracing_off=none was specified on the commandline. >> >> >> >> Signed-off-by: Chase Douglas <chase.douglas(a)canonical.com> >> >> --- >> >> �kernel/sched.c | � �2 ++ >> >> �1 files changed, 2 insertions(+), 0 deletions(-) >> >> >> >> diff --git a/kernel/sched.c b/kernel/sched.c >> >> index 6af210a..439f036 100644 >> >> --- a/kernel/sched.c >> >> +++ b/kernel/sched.c >> >> @@ -3590,6 +3590,8 @@ static noinline void __schedule_bug(struct task_struct *prev) >> >> �{ >> >> � � � struct pt_regs *regs = get_irq_regs(); >> >> >> >> + � � tracing_off_event(TRACE_EVENT_BUG); >> >> + >> >> � � � printk(KERN_ERR "BUG: scheduling while atomic: %s/%d/0x%08x\n", >> >> � � � � � � � prev->comm, prev->pid, preempt_count()); >> > >> > >> > >> > I would rather call that a TRACE_EVENT_WARN as this is what happens: we >> > warn but we continue. >> >> I tend to think of the TRACE_EVENT_* as an indication of severity and >> whether we want to stop the trace by default. From a distro >> standpoint, the likelihood that we want to continue tracing after a >> __schedule_bug is pretty low. It's easiest if we don't have to tell > > Well, scheduling while atomic is a BUG, but one of the category which > allows the kernel to continue. So in fact it's treated like a WARN_ON. > So the tracing_off_event() qualifier should be *_WARN. > > That's independent of the question whether you want to stop tracing in > that very case. Though I agree that the tracer should stop here. We seem to be agreeing on the functionality. The disagreement seems to be in the macro name/functionality mapping. However, the name of the function itself is *_bug. I don't see how things are clearer or more useful by inserting a *_WARN level macro in a *_bug named function. Essentially, it makes more sense to me for the macro to represent the severity of the case, and not be coupled somehow to what the kernel decides to do outside of the tracing. Is it the case that you really feel it should be *_WARN, or that the macro name/functionality should be different? -- Chase -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Next
|
Last
Pages: 1 2 3 Prev: reiserfs locking Next: [PATCH] Staging: dream: smd: smd_qmi: fix code style issues |