[PATCH v6 13/13] drm/print: Add tracefs support to the drm logging helpers

[Sean Paul]

On Wed, Jun 10, 2020 at 11:01 AM Steven Rostedt <rostedt at goodmis.org> wrote:
>
> On Tue,  9 Jun 2020 11:49:19 -0400
> Sean Paul <sean at poorly.run> wrote:
>
> > +/**
> > + * drm_trace_printf - adds an entry to the drm tracefs instance
> > + * @format: printf format of the message to add to the trace
> > + *
> > + * This function adds a new entry in the drm tracefs instance
> > + */
> > +void drm_trace_printf(const char *format, ...)
> > +{
> > +     struct va_format vaf;
> > +     va_list args;
> > +
> > +     va_start(args, format);
> > +     vaf.fmt = format;
> > +     vaf.va = &args;
> > +     trace_array_printk(trace_arr, _THIS_IP_, "%pV", &vaf);
> > +     va_end(args);
> > +}
>
> The only issue I have with this patch is the above. I really dislike
> the use of trace_array_print(), as that is made to be for debugging and
> not something that should be in a production kernel.
>
> Ideally, every instance should just pass the data it wants to record,
> and you add it to a trace event. There's already a drm trace subsystem,
> how would this be different. Perhaps create a drm_log subsystem, and
> you only need to have your instance enable it?
>
> I guess I'm still confused to why this is needed over just having trace
> events? What's special about this case?
>

Hi Steve,
In v2 I had added trace events throughout drm in this manner. See
https://lists.freedesktop.org/archives/dri-devel/2019-November/243318.html

The feedback I received was that exposing the events in such a
structured way would bind us to the trace formats and make things more
difficult moving forward.

Aside from the UAPI concerns, wearing my ChromeOS hat (with full
knowledge that most upstream folks won't care about this), this
solution also misses on making problems easier to diagnose from
logs/user feedback. It requires that driver developers add tracing
beside their logging, and makes backporting upstream code even harder
than it is right now. If we can intercept the existing and new logging
at the source, it's much easier to ensure the flight recorder is
accurate. We've been shipping this (v4) in CrOS for ~months now and
it's been invaluable. With this experience in mind, I'd be really
hesitant to change course.

Sean

> -- Steve
>