Hi Daniel,
On Tue, 19 Jul 2022, 1:25am, Daniel Wagner wrote:
> External Email
>
> ----------------------------------------------------------------------
> 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@xxxxxxxxxxx> 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.
No contention here on a generic way. The per instance trace creation and
enabling from within the kernel looks like such a one. Let me revisit the
trace patches with this new info.
Regards,
-Arun
>
> > 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
>
