Re: [PATCH 2/6] qla2xxx: Add a generic tracing framework

[Daniel Wagner <dwagner@suse.de>]

Hi Arun,

On Mon, Jul 18, 2022 at 03:22:43PM -0400, Steven Rostedt wrote:
> On Mon, 18 Jul 2022 12:02:26 -0700
> Arun Easi <aeasi@marvell.com> wrote:
> 
> > Many times when a problem was reported on the driver, we had to request 
> > for a repro with extended error logging (via driver module parameter) 
> > turned on. With this internal tracing in place, log messages that appear 
> > only with extended error logging are captured by default in the internal 
> > trace buffer.
> > 
> > AFAIK, kernel tracing requires a user initiated action to be turned on, 
> > like enabling individual tracepoints. Though a script (either startup or 
> > udev) can do this job, enabling tracepoints by default for a single 
> > driver, I think, may not be a preferred choice -- particularly when the 
> > trace buffer is shared across the kernel. That also brings the extra 
> > overhead of finding how this could be done across various distros.
> > 
> > For cases where the memory/driver size matters, there is an option to 
> > compile out this feature, plus choosing a lower default trace buffer size.

I am really asking the question why do we need to add special debugging
code to every single driver? Can't we try to make more use of generic
code and extend it if necessary?

Both FC drivers qla2xxx and lpfc are doing their own thing for
debugging/logging and I really fail to see why we can't not move towards
a more generic way. Dumping logs to the kernel log was the simplest way
but as this series shows, this is not something you can do in production
systems.

> You can enable an ftrace instance from inside the kernel, and make it a
> compile time option.
>
> 	#include <linux/trace_events.h>
> 	#include <linux/trace.h>
> 
> 	struct trace_array *tr;
> 
> 	tr = trace_array_get_by_name("qla2xxx");
> 	trace_array_set_clr_event(tr, "qla", NULL, true);
> 
> And now you have trace events running:
> 
>  # cat /sys/kernel/tracing/instances/qla/trace

Thanks Steve for the tip!

Daniel