lkml.org 
[lkml]   [2008]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] ftrace: Add debug_print trace to print data from kernel to userspace
On Fri, Nov 14, 2008 at 12:21:20PM +0100, Frédéric Weisbecker wrote:
> Hi Aneesh!
>
> 2008/11/14 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>:
> > The trace add a new interface debug_print() which can be used
> > to dump data from kernel to user space using ftrace framework.
>
>
> The actual "nop tracer" which is the default selected (if trace_boot
> is not configured) let
> the tracing engine able to receive and handle TRACE_PRINT events.
> Even if nop tracer doesn't handle its output itself, the TRACE_PRINT
> event output is relayed by
> print_trace_fmt() in trace.c
>
> Your output does almost the same but it is already implemented.

We also want to make sure dp_printk doesn't do anything when tracer
is disabled. We do

int do_dp_printk(const char *fmt, ...)
{
int ret;
va_list args;
if (!tracer_enabled)
return 0;
.........
.......
>
>
>
> > +static void dp_trace_ctrl_update(struct trace_array *tr)
> > +{
> > + /* When starting a new trace, reset the buffers */
> > + if (tr->ctrl)
> > + start_dp_trace(tr);
> > + else
> > + stop_dp_trace(tr);
> > +}
>
>
> BTW, ctrl_update() have been removed very recently.
> Perhaps are you implementing this against the mainline? Its a better idea to
> submit a new tracer against latest -tip tree.

Yes the patches are against mainline.

>
> > +
> > +#ifdef CONFIG_NOP_TRACER
> > +int
> > +trace_selftest_startup_dp(struct tracer *trace, struct trace_array *tr)
> > +{
> > + /* What could possibly go wrong? */
> > + return 0;
> > +}
> > +#endif
>
>
> CONFIG_NOP_TRACER ?
> Wouldn't it rather depend on CONFIG_FTRACE_SELFTEST? :-)
>
> That would perhaps have been better to raise a ftrace_printk to make a test.
> If you don't do anything in your selftest, then it is unnecessary to
> implement one for
> your tracer.

I dropped the selftest callback.

>
>
> > +static enum print_line_t debug_print_line(struct trace_iterator *iter)
> > +{
> > + struct trace_seq *s = &iter->seq;
> > + struct trace_entry *entry;
> > +
> > + entry = iter->ent;
> > + switch (entry->type) {
> > + case TRACE_PRINT: {
> > + struct print_entry *field;
> > + trace_assign_type(field, entry);
> > +
> > + trace_seq_printf(s, "%s", field->buf);
> > + if (entry->flags & TRACE_FLAG_CONT)
> > + trace_seq_print_cont(s, iter);
> > + break;
>
>
> You should test if trace_seq_printf successed to print the whole line.
> If not, that's better to return TRACE_TYPE_PARTIAL_LINE, then the
> output will be retried later.

I am looking at this. I don't see TRACE_TYPE_PARTIAL_LINE in other
places in the code. Will look more.

>
>
>
> > + }
> > + default:
> > + printk(KERN_INFO "Unsupported type in debug_print\n");
> > + return TRACE_TYPE_UNHANDLED;
> > + }
> > +
> > + return TRACE_TYPE_HANDLED;
> > +}
> > +
> > +struct tracer dp_trace __read_mostly =
> > +{
> > + .name = "debug_print",
> > + .init = dp_trace_init,
> > + .reset = dp_trace_reset,
> > + .ctrl_update = dp_trace_ctrl_update,
> > +#ifdef CONFIG_FTRACE_SELFTEST
> > + .selftest = trace_selftest_startup_dp,
> > +#endif
> > + .print_line = debug_print_line,
> > +};
> > +
> > +__init static int init_dp_trace(void)
> > +{
> > + return register_tracer(&dp_trace);
> > +}
> > +device_initcall(init_dp_trace);
>
>
> Primarily, such a debug tracer is not a bad idea, IMHO.
> And note that all of I just wrote in this answer in only my opinion.
> Perhaps other people
> would find other uses of this tracer that actual default output of the
> tracing engine doesn't handle well, or trace.c is
> would not be the right place for further enhancements that could
> happen on debug entries if you need to.
>
> But the actual simple output that you are submitting along this tracer
> is already handled by the default output of the tracing internals.

What I wanted to get was a dmesg style output. The default output will
add pid and other information. That is why i did a print_line callback.
I also wanted to drop the header in the trace file. I didn't find a way
to do that.

-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-11-14 14:55    [from the cache]
©2003-2011 Jasper Spaans