Re: [PATCH v9 1/2] PCI: trace: Add a generic RAS tracepoint for hotplug event

Shuai Xue
2025-07-28T09:05:28Z

Hi, Bjorn,

在 2025/7/26 05:09, Bjorn Helgaas 写道:
> On Wed, Jul 23, 2025 at 11:31:07AM +0800, Shuai Xue wrote:
>> Hotplug events are critical indicators for analyzing hardware health,
>> and surprise link downs can significantly impact system performance and
>> reliability.
>>
>> Define a new TRACING_SYSTEM named "pci", add a generic RAS tracepoint
>> for hotplug event to help health checks. Add enum pci_hotplug_event in
>> include/uapi/linux/pci.h so applications like rasdaemon can register
>> tracepoint event handlers for it.
>>
>> The output is like below:
>>
>> $ echo 1 > /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
>> $ cat /sys/kernel/debug/tracing/trace_pipe
>> <...>-206 [001] ..... 40.373870: pci_hp_event: 0000:00:02.0 slot:10, event:LINK_DOWN
>>
>> <...>-206 [001] ..... 40.374871: pci_hp_event: 0000:00:02.0 slot:10, event:CARD_NOT_PRESENT
>
> I asked about documentation earlier [1], but didn't see any response.
> I think these tracepoints are important and will be widely used, so it
> seems like some kind of user guide would be helpful.

Sorry for missing your earlier email about documentation.

> Is there any convention for documenting tracepoints somewhere? It
> looks like there's some doc in Documentation/trace/? Should we be
> adding something there?

Regarding tracepoint documentation conventions, you raise a good point.
Looking at Documentation/trace/, most of the existing documentation
focuses on the tracing infrastructure itself rather than individual
tracepoint events.

For tracepoint events like the ones I'm familiar with (aer_event,
memory_failure_event, mce_event, mc_event), the typical approach has
been:

- Self-documenting through code - The TRACE_EVENT() definitions in
include/trace/events/ serve as the primary specification
- UAPI headers - Enums and structures in include/uapi/ provide the
interface definitions
- Commit messages - Detailed explanations of when/why events are
generated

However, there are some exceptions where specific events do have
dedicated documentation:

- Documentation/trace/events-power.rst - Power management events
- Documentation/trace/events-kmem.rst - Kernel memory allocation events
- Documentation/trace/events-nmi.rst - NMI events
- Documentation/trace/events-msr.rst - MSR (Model Specific Register) events

Given your point about these PCI tracepoints potentially being widely
used, I think adding documentation would be valuable. Bellow is the RFC
doc, are you happy with this?

diff --git a/Documentation/trace/events-pci.rst b/Documentation/trace/events-pci.rst
new file mode 100644
index 000000000000..f2f7cacba862
--- /dev/null
+++ b/Documentation/trace/events-pci.rst
@@ -0,0 +1,72 @@
+===========================
+Subsystem Trace Points: PCI
+===========================
+
+Overview
+========
+The PCI tracing system provides tracepoints to monitor critical hardware events
+that can impact system performance and reliability. These events normally show
+up here:
+
+ /sys/kernel/tracing/events/pci
+
+Cf. include/trace/events/pci.h for the events definitions.
+
+Available Tracepoints
+=====================
+
+pci_hp_event
+------------
+
+Monitors PCI hotplug events including card insertion/removal and link
+state changes.
+::
+
+ pci_hp_event "%s slot:%s, event:%s\n"
+
+**Event Types**:
+
+* ``LINK_UP`` - PCIe link established
+* ``LINK_DOWN`` - PCIe link lost
+* ``CARD_PRESENT`` - Card detected in slot
+* ``CARD_NOT_PRESENT`` - Card removed from slot
+
+**Example Usage**:
+
+ # Enable the tracepoint
+ echo 1> /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
+
+ # Monitor events
+ cat /sys/kernel/debug/tracing/trace_pipe
+ <...>-206 [001] ..... 40.373870: pci_hp_event: 0000:00:02.0 slot:10, event:LINK_DOWN
+
+ <...>-206 [001] ..... 40.374871: pci_hp_event: 0000:00:02.0 slot:10, event:CARD_NOT_PRESENT
+
+
+pcie_link_event
+---------------
+
+Monitors PCIe link speed changes and provides detailed link status information.
+::
+
+ pcie_link_event "%s type:%d, reason:%d, cur_bus_speed:%s, max_bus_speed:%s, width:%u, flit_mode:%u, status:%s\n"
+
+**Parameters**:
+
+* ``type`` - PCIe device type (4=Root Port, etc.)
+* ``reason`` - Reason for link change:
+
+ - ``0`` - Link retrain
+ - ``1`` - Bus enumeration
+ - ``2`` - Bandwidth controller enable
+ - ``3`` - Bandwidth controller IRQ
+ - ``4`` - Hotplug event
+
+
+**Example Usage**::
+
+ # Enable the tracepoint
+ echo1 > /sys/kernel/debug/tracing/events/pci/pcie_link_event/enable
+
+ # Monitor link events
+ cat /sys/kernel/debug/tracing/trace_pipe

>
>> Suggested-by: Lukas Wunner <lukas@wunner.de>
>> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Nit: I assume this came from the patch I had applied to pci/trace, but
> you shouldn't include any sign-offs from people to whom you send
> patches [2].

Yep, I copied the commit log from your applied patch in pci/trace. I
will drop your sign-offs.

>
> Bjorn
>
> [1] https://lore.kernel.org/all/20250717192950.GA2594528@bhelgaas/#t
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.13#n449

Thanks for the guidance!

Best regards,
Shuai