Re: [PATCH v1 1/2] thunderbolt: add tracefs support to tb_* logging helpers

[Greg KH]

On Fri, Jul 28, 2023 at 04:50:12PM +0200, Łukasz Bartosik wrote:
> czw., 27 lip 2023 o 16:47 Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> napisał(a):
> >
> > On Thu, Jul 27, 2023 at 03:13:25PM +0200, Łukasz Bartosik wrote:
> > > Thunderbolt tracing is a lightweight alternative to traditional
> > > thunderbolt debug logging. While thunderbolt debug logging is quite
> > > convenient when reproducing a specific issue, it doesn't help when
> > > something goes wrong unexpectedly. There are a couple of reasons why
> > > one does not want to enable thunderbolt debug logging at all times:
> > >
> > > 1. We don't want to overwhelm kernel log with thunderbolt spam, others
> > >    want to use it too
> >
> > But doesn't the dynamic debug infrastructure allow this today?
> >
> 
> Dynamic debug allows only partially what we would like to achieve.
> 
> Our goal is to be able to gather thunderbolt debug logs from ChromeOS
> end users when an issue happens. Especially that would be very useful
> for us in case of issues which reproduction is difficult. We would
> write thunderbolt debug logs to a separate wrap around buffer provided
> by trace subsystem. When an issue happens and a user sends a feedback
> then thunderbolt logs would be attached to the feedback providing
> more insight into what happened.

The problem is, you don't want 100 different debug log formats and tools
for the 100 different driver subsystems.

That is why we unified everything on the dev_* format, and the tracing
tools.  Use them, don't create something new.

> Dynamic debug sends all debug messages to dmesg and with significant
> number of dynamic dev_dbg prints enabled dmesg may be quickly overwritten
> and other important logs lost. Also enabling dynamic debug for the
> entire kernel might
> not be appropriate for production kernels due to impact on kernel size and perf.

If you look at the typec code, they have their own type of ring-buffer
for logging things.  Perhaps look at making that more generic like what
you need here as well, and adding that to the tracing core for everyone
to use and unify with?

I don't want one-off solutions like this, sorry, that way lies madness,
madness that we used to have before we unified everything.  Let's learn
from our past mistakes and not make them again please.

thanks,

greg k-h