Stop tracing on a schedule bug
in [Kernel]
|
From: Thomas Gleixner on 15 Apr 2010 20:00 On Thu, 15 Apr 2010, Chase Douglas wrote: > 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. Care to read what I wrote ? Again: > > 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. It does not matter at all whether the function name has "bug" in it or not. What matters is the semantics of the function. It does _NOT_ raise a BUG. It merily warns and tries to continue. So it follows the WARN() semantics. If you feel strong about that send a patch to s/schedule_bug/schedule_warn/ and I'll ack it. > 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. Essentially you are wrong. The semantic of schedule_bug() is clearly WARN() and not BUG(). So the tracing off qualifier needs to be WARN. And it does not matter what you consider as the severity. The severity is given by the semantics of schedule_bug(). If your extra stupid grub hiding logic prevents an user to change the trace off level then you need to fix that instead of anything else. BTW, if interacting with grub is that hard: how does an user start the tracer at all ? Thanks, tglx
From: Chase Douglas on 16 Apr 2010 00:00 On Thu, Apr 15, 2010 at 4:50 PM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > On Thu, 15 Apr 2010, Chase Douglas wrote: >> 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. > > Care to read what I wrote ? Again: I misunderstood what you wrote. I thought you were agreeing that the tracing should stop by default in __schedule_bug. I now realize you meant that you agree it should be possible to stop it, but it should not be the default behavior. Sorry for the confusion. I will change the event to be a *_WARN type. -- 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: Steven Rostedt on 16 Apr 2010 13:00 On Thu, 2010-04-15 at 21:01 -0700, Chase Douglas wrote: > > 2) tracing off can be done via filters on functions and/or events > > already - so I doubt that the tracing_off_event(level) is necessary > > at all. > > > > schedule_bug() definitely deserves a separate trace_schedule_bug() > > event which can be used to stop the tracer by already existing > > functionality. > > Steven said he would be fine with a separate TRACE_EVENT_<blah> macro > for the schedule bug if needed, but I'm not sure we need to go that > far. If it's configurable through debugfs at run time then it serves > my purpose. Unless you feel we should have finer grained control > specifically for scheduling while atomic bugs, I'll just leave it as > TRACE_EVENT_WARN. I actually like Thomas's idea better. I need to add the "stop trace on event" functionality, and we can insert trace events for bugs, and not have this whole "stop tracing here" functions. Instead we could just add tracepoints and have a way to pick and choose where to stop tracing. add a: include/trace/events/errors.h #define TRACE_SYSTEM errors TRACE_EVENT(sched_bug, ....) etc, When I get back home, I'll add this functionality to stop tracing on events. Perhaps I'll even add "TRACE_SUB_SYSTEM" so in the events directory, we can have sub layers: events/errors/BUG/... events/errors/WARNING/... etc -- Steve -- 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 16 Apr 2010 13:20 On Fri, Apr 16, 2010 at 9:46 AM, Steven Rostedt <rostedt(a)goodmis.org> wrote: > On Thu, 2010-04-15 at 21:01 -0700, Chase Douglas wrote: >> > 2) tracing off can be done via filters on functions and/or events >> > � already - so I doubt that the tracing_off_event(level) is necessary >> > � at all. >> > >> > � schedule_bug() definitely deserves a separate trace_schedule_bug() >> > � event which can be used to stop the tracer by already existing >> > � functionality. >> >> Steven said he would be fine with a separate TRACE_EVENT_<blah> macro >> for the schedule bug if needed, but I'm not sure we need to go that >> far. If it's configurable through debugfs at run time then it serves >> my purpose. Unless you feel we should have finer grained control >> specifically for scheduling while atomic bugs, I'll just leave it as >> TRACE_EVENT_WARN. > > I actually like Thomas's idea better. I need to add the "stop trace on > event" functionality, and we can insert trace events for bugs, and not > have this whole "stop tracing here" functions. Instead we could just add > tracepoints and have a way to pick and choose where to stop tracing. > > add a: > > include/trace/events/errors.h > > #define TRACE_SYSTEM errors > > TRACE_EVENT(sched_bug, ....) > > etc, > > > When I get back home, I'll add this functionality to stop tracing on > events. Perhaps I'll even add "TRACE_SUB_SYSTEM" so in the events > directory, we can have sub layers: > > events/errors/BUG/... > events/errors/WARNING/... > > etc Ok, I see where this is going now. I agree this sounds like a much better approach. I'll wait for the implementation of the stop tracing on event functionality so I know how it will work, and then I can submit patches for the BUG, WARN, panic, and schedule_bug paths. -- 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: Frederic Weisbecker on 16 Apr 2010 14:20 On Fri, Apr 16, 2010 at 12:46:53PM -0400, Steven Rostedt wrote: > On Thu, 2010-04-15 at 21:01 -0700, Chase Douglas wrote: > > > > 2) tracing off can be done via filters on functions and/or events > > > already - so I doubt that the tracing_off_event(level) is necessary > > > at all. > > > > > > schedule_bug() definitely deserves a separate trace_schedule_bug() > > > event which can be used to stop the tracer by already existing > > > functionality. > > > > Steven said he would be fine with a separate TRACE_EVENT_<blah> macro > > for the schedule bug if needed, but I'm not sure we need to go that > > far. If it's configurable through debugfs at run time then it serves > > my purpose. Unless you feel we should have finer grained control > > specifically for scheduling while atomic bugs, I'll just leave it as > > TRACE_EVENT_WARN. > > I actually like Thomas's idea better. I need to add the "stop trace on > event" functionality, and we can insert trace events for bugs, and not > have this whole "stop tracing here" functions. Instead we could just add > tracepoints and have a way to pick and choose where to stop tracing. > > add a: > > include/trace/events/errors.h > > #define TRACE_SYSTEM errors > > TRACE_EVENT(sched_bug, ....) > > etc, > Looks good. > When I get back home, I'll add this functionality to stop tracing on > events. Perhaps I'll even add "TRACE_SUB_SYSTEM" so in the events > directory, we can have sub layers: > > events/errors/BUG/... > events/errors/WARNING/... I agree this could be nice. I'm thinking about the printk all around the kernel that can be represented as trace events with sub-sub-systems that could be file paths of the kernel. I just hope we can find a way to do this that won't break tools. -- 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/
First
|
Prev
|
Next
|
Last
Pages: 1 2 3 Prev: reiserfs locking Next: [PATCH] Staging: dream: smd: smd_qmi: fix code style issues |