Re: [PATCH v9 2/2] PCI: trace: Add a RAS tracepoint to monitor link speed changes

[Shuai Xue <xueshuai@linux.alibaba.com>]

Hi, Steve

在 2025/7/23 22:05, Steven Rostedt 写道:
> On Wed, 23 Jul 2025 11:31:08 +0800
> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> 
>> +	TP_printk("%s type:%d, reason:%d, cur_bus_speed:%s, max_bus_speed:%s, width:%u, flit_mode:%u, status:%s\n",
>> +		__get_str(port_name),
>> +		__entry->type,
>> +		__entry->reason,
>> +		pci_speed_string(__entry->cur_bus_speed),
>> +		pci_speed_string(__entry->max_bus_speed),
> 
> Hmm, I guess pci_speed_string() should be added to libtraceveent so
> that perf and trace-cmd parses it correctly. I guess rasdaemon would
> want that too (which also uses libtraceevent).

Thank you for pointing this out. You're absolutely right that
pci_speed_string() should be properly handled in libtraceevent for
better userspace tool support.

$ cat /sys/kernel/debug/tracing/trace_pipe

    irq/57-pciehp-119     [002] .....   125.904335: pcie_link_event: 0000:00:03.0 type:4, reason:4, cur_bus_speed:2.5 GT/s PCIe, max_bus_speed:16.0 GT/s PCIe, width:1, flit_mode:0, status:DLLLA

    irq/57-pciehp-119     [002] .....   125.907051: pcie_link_event: 0000:00:03.0 type:4, reason:0, cur_bus_speed:2.5 GT/s PCIe, max_bus_speed:16.0 GT/s PCIe, width:1, flit_mode:0, status:DLLLA

Compared with debug/tracing, perf trace used the raw event field, and
the speed is not handman readable.

$ perf trace -e pci:pcie_link_event
      0.000 irq/57-pciehp/121 pci:pcie_link_event(port_name: "0000:00:03.0", type: 4, reason: 4, cur_bus_speed: 20, max_bus_speed: 23, width: 1, link_status: 8192)
      4.058 irq/57-pciehp/121 pci:pcie_link_event(port_name: "0000:00:03.0", type: 4, cur_bus_speed: 20, max_bus_speed: 23, width: 1, link_status: 8192)

I see a couple of options here:

1. Keep the current approach and add libtraceevent support as follow-up
work. The tracepoint would still be functional, but userspace tools
would show raw speed values instead of formatted strings until
libtraceevent is updated.

2. Use raw values in the tracepoint for now (e.g., store speed as
integer) and let userspace tools handle the formatting. This would avoid
the immediate dependency on libtraceevent updates.

3. Address both kernel and userspace in coordinated patch set.

Which approach would you prefer? If you think option1 is acceptable, I'm
happy to work on the libtraceevent changes as a follow-up.
Alternatively, if you'd prefer option 2, I can modify the tracepoint to
use raw values. And if you perfer opiton 3, I will also include a new
patch 3 to add a plugin helper for libtraceevent.


For the libtraceevent implementation, I believe we'd
need to:

- Add the PCI speed mapping table to libtraceevent
- Create a print function similar to other existing parsers
- Ensure perf, trace-cmd, and rasdaemon can all benefit from it

Would you like me to investigate the libtraceevent changes, or do you
have other suggestions for the approach?

Thanks again for the feedback.

Best regards,
Shuai


> 
> -- Steve
> 
> 
>> +		__entry->width,
>> +		__entry->flit_mode,
>> +		__print_flags((unsigned long)__entry->link_status, "|",
>> +				LNKSTA_FLAGS)
>> +	)
>> +);