Re: [PATCH] tracing: add tracing C helper functions

[Steven Rostedt]

Sorry for the late reply. This got lost in my INBOX, and I'm just now
noticing it. If you send me a patch and don't hear a response, feel
free to send me a "ping" to ask about the progress. I never just
"ignore" a patch. If you don't hear from me, it's usually because
I put it in my "todo" and it got pushed down the priority list
(as it gets buried with other patches in the todo list, and then I
forget it's there), or you sent it while I was traveling or on holiday
(it was the day after Thanksgiving when you sent this, and I was too
busy with my Black Friday shopping!).

Anyway, I have no issue with the addition. Just some notes.


On Fri, 27 Nov 2015 15:53:44 +0200
Ioan-Adrian Ratiu <adrian.ratiu@xxxxxx> wrote:

> Tracing/debugging programatically from kernel C code is quite limited
> compared to its tracefs alternative mainly due to lack of interfaces.
> linux/kernel.h provides traceing_on/off() and *_snapshot(), but these
> only allow access to a limited subset of the tracing functionality.
> 
> This patch adds 5 new functions to make tracing from C easier:
> 	tracing_change_tracer - change the live tracer
> 	tracing_clear_all - clear all cpu ringbuffers
> 	tracing_clear_cpu - clear a specific cpu's buffer
> 	tracing_resize_all - set ringbuffer size for all cpus

Perhaps name this tracing_resize_kb_all() to note that it still uses
kilobytes as a measurement.

> 	tracing_resize_cpu - set ringbuffer size for specific cpu

tracing_resize_kb_cpu() as above.

> 
> These functions are very helpful when combined with existing ones, for
> example to debug the call serial8250_pnp_init() in serial8250_init():
> 
> 	tracing_resize_all(1 << 23);

This should definitely return a result, because resizing can fail.


> 	tracing_change_tracer("function_graph");

Same here.

Although I'm not sure why you would want to change the tracer, as that
would zero out the buffers of the last trace. You can't just change
tracers and have more than one tracer in the result.

I actually find it hard to think of a good use case for
change_tracer(). I don't think its needed. It can be easily done by
userspace, and if you need it enabled at boot up, just set it via a
cmdline kernel boot parameter.

Is there something I'm missing?

> 	tracing_on();
> 
> 	trace_printk("starting pnp init\n");
> 	ret = serial8250_pnp_init();
> 	trace_printk("done pnp init\n");
> 
> 	tracing_off();
> 	tracing_clear_cpu((1 - smp_processor_id());

If we do have a trace_clear_cpu(), it should be able to work with:

 tracing_clear_cpu(smp_processor_id());

The function needs to handle the 1-X part.

The above example doesn't make sense. You write to the buffer, turn off
tracing, then clear it? You lose out on what you write. I can see
perhaps to clear it before the tracing_on(). This can be handy if you
are also using ftrace_dump_on_oops() if you know where the problem
starts and you don't want to wait forever for serial output of printing
traces you don't care about.

Note, I'm on holiday after today and wont be back till Jan 4th. So I
can probably miss replies to this email ;-)

-- Steve



> 
> or conditionally trace driver_register() only for the serial driver:
> 
> 	if (!strncmp(drv->name, "serial", 6)) {
> 		tracing_change_tracer("function_graph");
> 		tracing_on();
> 	}
> 
> Signed-off-by: Ioan-Adrian Ratiu <adrian.ratiu@xxxxxx>
> ---
>  include/linux/kernel.h |  5 +++++
>  kernel/trace/trace.c   | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index ae6adf8..7cd24dd 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -552,6 +552,11 @@ void tracing_off(void);
>  int tracing_is_on(void);
>  void tracing_snapshot(void);
>  void tracing_snapshot_alloc(void);
> +void tracing_clear_all(void);
> +void tracing_clear_cpu(int cpu);
> +void tracing_resize_all(unsigned long size);
> +void tracing_resize_cpu(unsigned long size, int cpu);
> +void tracing_change_tracer(const char *tracer);
>  
>  extern void tracing_start(void);
>  extern void tracing_stop(void);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 87fb980..a8ed37e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -533,6 +533,62 @@ void tracing_on(void)
>  EXPORT_SYMBOL_GPL(tracing_on);
>  
>  /**
> + * tracing_change_tracer - switch live tracing to specified tracer
> + * @tracer: Name of the tracer to change to
> + */
> +void tracing_change_tracer(const char *tracer)
> +{
> +	tracing_set_tracer(&global_trace, tracer);
> +}
> +EXPORT_SYMBOL_GPL(tracing_change_tracer);
> +
> +/**
> + * tracing_clear_cpu - reset a cpu live tracing buffer
> + * @cpu: cpu to clear
> + */
> +void tracing_clear_cpu(int cpu)
> +{
> +	mutex_lock(&trace_types_lock);
> +	tracing_reset(&global_trace.trace_buffer, cpu);
> +	mutex_unlock(&trace_types_lock);
> +}
> +EXPORT_SYMBOL_GPL(tracing_clear_cpu);
> +
> +/**
> + * tracing_clear_all - reset all cpu tracing buffers
> + */
> +void tracing_clear_all(void)
> +{
> +	mutex_lock(&trace_types_lock);
> +	tracing_reset_online_cpus(&global_trace.trace_buffer);
> +	mutex_unlock(&trace_types_lock);
> +}
> +EXPORT_SYMBOL_GPL(tracing_clear_all);
> +
> +static ssize_t tracing_resize_ring_buffer(struct trace_array *, unsigned long, int);
> +
> +/**
> + * tracing_resize_cpu - resize a cpu's ring buffer
> + * @size - new size in KB
> + * @cpu - target cpu for buf resize
> + */
> +void tracing_resize_cpu(unsigned long size, int cpu)
> +{
> +	tracing_resize_ring_buffer(&global_trace, size, cpu);
> +}
> +EXPORT_SYMBOL_GPL(tracing_resize_cpu);
> +
> +/**
> + * tracing_resize_all - resize all per-cpu ring buffers
> + * @size - new size in KB
> + */
> +void tracing_resize_all(unsigned long size)
> +{
> +	tracing_resize_ring_buffer(&global_trace, size, RING_BUFFER_ALL_CPUS);
> +}
> +EXPORT_SYMBOL_GPL(tracing_resize_all);
> +
> +/**
>   * __trace_puts - write a constant string into the trace buffer.
>   * @ip:	   The address of the caller
>   * @str:   The constant string to write

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