tracing: Allow mmio tracer to display trace_printk() and other events
in [Kernel]
|
From: Pekka Paalanen on 13 May 2010 05:00 On Wed, 12 May 2010 21:21:13 -0400 Steven Rostedt <rostedt(a)goodmis.org> wrote: > From: Steven Rostedt <srostedt(a)redhat.com> > > The mmio tracer has its own function to handle reading of events. > But if it encounters an event that it does not understand it > ignores it instead of telling the calling function that it is not > processing it. > > If someone adds trace_printk() or enables events along with the > mmio tracer, then these events will not be displayed in the trace > output. > > Simple solution is to just have the mmio print return UNHANDLED to > let the caller know that it did not processes the event and the > caller can process the event further. Does this not mean that the mmiotrace output may contain foreign lines? If it does, it will break the user space. The dump format is specified in Documentation/trace/mmiotrace.txt. If you want to handle arbitrary messages, format them as MARK events, please. If I understood correctly, then NAK for this patch. Otherwise, could you explain how this does not break the mmiotrace dump format? Is the tracing infrastructure now supporting several active tracers at the same time? If yes, and if mmiotrace should be able to co-operate, we need a new revision of the dump format, or a tool that extracts the mmiotrace events in the current format. Thanks. > Reported-by: Larry Finger <Larry.Finger(a)lwfinger.net> > Cc: Pekka Paalanen <pq(a)iki.fi> > Signed-off-by: Steven Rostedt <rostedt(a)goodmis.org> > --- > kernel/trace/trace_mmiotrace.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/kernel/trace/trace_mmiotrace.c > b/kernel/trace/trace_mmiotrace.c index 017fa37..592c00f 100644 > --- a/kernel/trace/trace_mmiotrace.c > +++ b/kernel/trace/trace_mmiotrace.c > @@ -282,7 +282,8 @@ static enum print_line_t > mmio_print_line(struct trace_iterator *iter) case TRACE_PRINT: > return mmio_print_mark(iter); > default: > - return TRACE_TYPE_HANDLED; /* ignore unknown > entries */ > + /* Not our event */ > + return TRACE_TYPE_UNHANDLED; > } > } > > -- > 1.7.0 -- Pekka Paalanen http://www.iki.fi/pq/ -- 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 13 May 2010 08:20 On Thu, 2010-05-13 at 11:54 +0300, Pekka Paalanen wrote: > On Wed, 12 May 2010 21:21:13 -0400 > Steven Rostedt <rostedt(a)goodmis.org> wrote: > > > From: Steven Rostedt <srostedt(a)redhat.com> > > > > The mmio tracer has its own function to handle reading of events. > > But if it encounters an event that it does not understand it > > ignores it instead of telling the calling function that it is not > > processing it. > > > > If someone adds trace_printk() or enables events along with the > > mmio tracer, then these events will not be displayed in the trace > > output. > > > > Simple solution is to just have the mmio print return UNHANDLED to > > let the caller know that it did not processes the event and the > > caller can process the event further. > > Does this not mean that the mmiotrace output may contain > foreign lines? If it does, it will break the user space. > The dump format is specified in > Documentation/trace/mmiotrace.txt. > > If you want to handle arbitrary messages, format them as > MARK events, please. > > If I understood correctly, then NAK for this patch. Otherwise, > could you explain how this does not break the mmiotrace dump > format? > > Is the tracing infrastructure now supporting several > active tracers at the same time? If yes, and if mmiotrace > should be able to co-operate, we need a new revision of the > dump format, or a tool that extracts the mmiotrace > events in the current format. > It only displays other events if the user enabled those events. But that said, I don't want to break existing userspace tools. I can add a mmiotrace option "mmiotrace_all_events", if the user wants to see all events within the mmiotracer then they can just enable that option, otherwise, the mmiotracer will act like it currently does. How does that sound? -- Steve > Thanks. > > > Reported-by: Larry Finger <Larry.Finger(a)lwfinger.net> > > Cc: Pekka Paalanen <pq(a)iki.fi> > > Signed-off-by: Steven Rostedt <rostedt(a)goodmis.org> > > --- > > kernel/trace/trace_mmiotrace.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/kernel/trace/trace_mmiotrace.c > > b/kernel/trace/trace_mmiotrace.c index 017fa37..592c00f 100644 > > --- a/kernel/trace/trace_mmiotrace.c > > +++ b/kernel/trace/trace_mmiotrace.c > > @@ -282,7 +282,8 @@ static enum print_line_t > > mmio_print_line(struct trace_iterator *iter) case TRACE_PRINT: > > return mmio_print_mark(iter); > > default: > > - return TRACE_TYPE_HANDLED; /* ignore unknown > > entries */ > > + /* Not our event */ > > + return TRACE_TYPE_UNHANDLED; > > } > > } > > > > -- > > 1.7.0 > -- 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: Pekka Paalanen on 13 May 2010 08:30 On Thu, 13 May 2010 08:15:09 -0400 Steven Rostedt <rostedt(a)goodmis.org> wrote: > On Thu, 2010-05-13 at 11:54 +0300, Pekka Paalanen wrote: > > On Wed, 12 May 2010 21:21:13 -0400 > > Steven Rostedt <rostedt(a)goodmis.org> wrote: > > > > > From: Steven Rostedt <srostedt(a)redhat.com> > > > > > > The mmio tracer has its own function to handle reading of > > > events. But if it encounters an event that it does not > > > understand it ignores it instead of telling the calling > > > function that it is not processing it. > > > > > > If someone adds trace_printk() or enables events along with > > > the mmio tracer, then these events will not be displayed in > > > the trace output. > > > > > > Simple solution is to just have the mmio print return > > > UNHANDLED to let the caller know that it did not processes > > > the event and the caller can process the event further. > > > > Does this not mean that the mmiotrace output may contain > > foreign lines? If it does, it will break the user space. > > The dump format is specified in > > Documentation/trace/mmiotrace.txt. > > > > If you want to handle arbitrary messages, format them as > > MARK events, please. > > > > If I understood correctly, then NAK for this patch. Otherwise, > > could you explain how this does not break the mmiotrace dump > > format? > > > > Is the tracing infrastructure now supporting several > > active tracers at the same time? If yes, and if mmiotrace > > should be able to co-operate, we need a new revision of the > > dump format, or a tool that extracts the mmiotrace > > events in the current format. > > > > It only displays other events if the user enabled those events. > > But that said, I don't want to break existing userspace tools. I > can add a mmiotrace option "mmiotrace_all_events", if the user > wants to see all events within the mmiotracer then they can just > enable that option, otherwise, the mmiotracer will act like it > currently does. > > How does that sound? That would be fine. Is it not redundant with what you said in your first sentence? -- Pekka Paalanen http://www.iki.fi/pq/ -- 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 13 May 2010 11:20 On Thu, 2010-05-13 at 15:29 +0300, Pekka Paalanen wrote: > On Thu, 13 May 2010 08:15:09 -0400 > > It only displays other events if the user enabled those events. > > > > But that said, I don't want to break existing userspace tools. I > > can add a mmiotrace option "mmiotrace_all_events", if the user > > wants to see all events within the mmiotracer then they can just > > enable that option, otherwise, the mmiotracer will act like it > > currently does. > > > > How does that sound? > > That would be fine. Is it not redundant with what you said in > your first sentence? > Right now with this patch as is. When you enable the mmiotracer it clears the ring buffer. But if someone previously enabled an event (like sched_switch for example) then that event will appear in the output of the tracer. The user will need to disable that event and restart the mmiotracer so the output will not break the userspace tools. Is this OK? If not, then my suggestion is to have an mmiotracer option that keeps it from printing out any event except for the ones it knows about. The reason I added this patch in the first place was because Larry Finger was using the mmiotrace with trace_printk() and the current code does not print out the trace_printk() when mmiotracer is active. -- 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: Steven Rostedt on 13 May 2010 15:50 On Thu, 2010-05-13 at 11:11 -0400, Steven Rostedt wrote: > > > How does that sound? > > > > That would be fine. Is it not redundant with what you said in > > your first sentence? > > > > Right now with this patch as is. When you enable the mmiotracer it > clears the ring buffer. But if someone previously enabled an event (like > sched_switch for example) then that event will appear in the output of > the tracer. > > The user will need to disable that event and restart the mmiotracer so > the output will not break the userspace tools. Is this OK? > > If not, then my suggestion is to have an mmiotracer option that keeps it > from printing out any event except for the ones it knows about. > > The reason I added this patch in the first place was because Larry > Finger was using the mmiotrace with trace_printk() and the current code > does not print out the trace_printk() when mmiotracer is active. Pekka, Are you OK with this version of the patch or do you want me to add the option as described above? -- 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/
|
Next
|
Last
Pages: 1 2 Prev: [GIT PULL] last minute vhost-net fix Next: ACPI, APEI, EINJ injection parameters support |