Linux USB
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "Łukasz Bartosik" <lb@semihalf.com>
Cc: Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	upstream@semihalf.com
Subject: Re: [PATCH v1 1/2] thunderbolt: add tracefs support to tb_* logging helpers
Date: Thu, 27 Jul 2023 16:46:56 +0200	[thread overview]
Message-ID: <2023072705-palpitate-cut-874b@gregkh> (raw)
In-Reply-To: <20230727131326.2282301-1-lb@semihalf.com>

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?

> 2. Console logging is slow

Slow how?

> Thunderbolt tracing aims to solve both these problems. Use of
> the thunderbolt tracefs instance allows to enable thunderbolt
> logging in production without impacting performance or spamming
> the system logs.
> 
> To use thunderbolt tracing, set the thunderbolt.trace module parameter
> (via cmdline or sysfs) to true:
> ::
>   eg: echo true > /sys/module/thunderbolt/parameters/trace

Why yet-another module parameter?  Why is this required?

> Once active, all log messages will be written to the thunderbolt trace.
> Once at capacity, the trace will overwrite old messages with new ones.
> At any point, one can read the trace file to extract the previous
> thunderbolt messages:
> ::
>   eg: cat /sys/kernel/tracing/instances/thunderbolt/trace
> 
> The thunderbolt trace instance is subsystem wide, so if you have multiple
> devices active, they will be adding logs to the same trace.

This just uses the existing logging functionality and ties it into the
trace functionality, right?

If so, why not do this for all printk messages, why should this be
unique to thunderbolt?

Normally with tracing, you enable specific tracepoints that you add, not
just "all dev_*() messages", are you sure this will actually help?

thanks,

greg k-h

  parent reply	other threads:[~2023-07-27 14:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-27 13:13 [PATCH v1 1/2] thunderbolt: add tracefs support to tb_* logging helpers Łukasz Bartosik
2023-07-27 13:13 ` [PATCH v1 2/2] thunderbolt: add tracefs support to dev_* " Łukasz Bartosik
2023-07-27 14:46 ` Greg KH [this message]
2023-07-28 14:50   ` [PATCH v1 1/2] thunderbolt: add tracefs support to tb_* " Łukasz Bartosik
2023-07-28 15:32     ` Greg KH
2023-07-28 15:49       ` Łukasz Bartosik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2023072705-palpitate-cut-874b@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=lb@semihalf.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michael.jamet@intel.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=upstream@semihalf.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox