[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240325004134.GA21329@workstation.local>
Date: Mon, 25 Mar 2024 09:41:34 +0900
From: Takashi Sakamoto <o-takashi@...amocchi.jp>
To: Adam Goldman <adamg@...ox.com>
Cc: linux1394-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] firewire: core: option to log bus reset initiation
Hi Adam,
On Wed, Mar 20, 2024 at 02:14:11AM -0700, Adam Goldman wrote:
> Add a debug parameter to firewire-core, analogous to the one in
> firewire-ohci. When this is set to 1, log when we schedule, delay, or
> initiate a bus reset. Since FireWire bus resets can originate from any
> node on the bus, specific logging of the resets we initiate provides
> additional insight.
>
> Signed-off-by: Adam Goldman <adamg@...ox.com>
Thanks for the patch. I applied it to for-next[1].
Now we have two debug parameters per module for the slightly-similar
purpose. In my opinion, it is a pretty cumbersome to enable them when
checking bus-reset behaviour. I think it is time to investigate the other
way.
Linux Kernel Tracepoints[2] is one of options. Roughly describing, the
tracepoints mechanism allows users to deliver structured data from kernel
space to user space via ring-buffer when enabling it by either sysfs or
kernel command-line parameters. Linux kernel also has a command-line
parameter to redirect the human-readable formatted data to kernel log[3].
I think it is suitable in the case.
It requires many work to replace the existent debug parameter of
firewire-ohci, while it is a good start to work just for bus-reset debug.
The data structure layout should be pre-defined in each subsystem, thus we
need to decide it. In my opinion, it would be like:
```
struct bus_reset_event {
enum reason {
Initiate,
Schedule,
Postpone,
Detect,
},
// We can put any other data if prefering.
}
```
Would I ask your opinion about my idea?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/log/?h=for-next
[2] https://docs.kernel.org/trace/tracepoints.html
[3] 'tp_printk' in kernel/trace/trace.c, 'trace_event' in kernel/trace/trace_event.c
Regards
Takashi Sakamoto
Powered by blists - more mailing lists