Re: [PATCH v8] PCI: hotplug: Add a generic RAS tracepoint for hotplug event

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Ilpo, Jonathan (should have been included since the patch has his
Reviewed-by)]

Thanks for the ping; I noticed quite a bit of discussion but didn't
follow it myself, so didn't know it was basically all resolved.

On Mon, May 12, 2025 at 09:38:39AM +0800, Shuai Xue wrote:
> Hotplug events are critical indicators for analyzing hardware health,
> particularly in AI supercomputers where surprise link downs can
> significantly impact system performance and reliability.

I dropped the "particularly in AI supercomputers" part because I think
this is relevant in general.

> To this end, define a new TRACING_SYSTEM named pci, add a generic RAS
> tracepoint for hotplug event to help healthy check, and generate
> tracepoints for pcie hotplug event.

I'm not quite clear on the difference between "add generic RAS
tracepoint for hotplug event" and "generate tracepoints for pcie
hotplug event."  Are these two different things?

I see the new TRACE_EVENT(pci_hp_event, ...) definition.  Is that what
you mean by the "generic RAS tracepoint"?

And the five new trace_pci_hp_event() calls that use the TRACE_EVENT
are the "tracepoints for PCIe hotplug event"?

> Add enum pci_hotplug_event in
> include/uapi/linux/pci.h so applications like rasdaemon can register
> tracepoint event handlers for it.
> 
> The output 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

> +#define PCI_HOTPLUG_EVENT					\
> +	EM(PCI_HOTPLUG_LINK_UP,			"Link Up")	\
> +	EM(PCI_HOTPLUG_LINK_DOWN,		"Link Down")	\
> +	EM(PCI_HOTPLUG_CARD_PRESENT,		"Card present")	\
> +	EMe(PCI_HOTPLUG_CARD_NOT_PRESENT,	"Card not present")

Running this:

  $ git grep -E "\<(EM|EMe)\("

I notice that these new events don't look like the others, which
mostly look like "word" or "event-type" or "VERB object".

I'm OK with this, but just giving you a chance to consider what will
be the least surprise to users and easiest for grep and shell
scripting.

I also noticed capitalization of "Up" and "Down", but not "present"
and "not present".

"Card" is only used occasionally and informally in the PCIe spec, and
not at all in the context of hotplug of Slot Status (Presence Detect
State refers to "adapter in the slot"), but it does match the pciehp
dmesg text, so it probably makes sense to use that.

Anyway, I applied this on pci/trace for v6.17.  If there's anything
you want to tweak in the commit log or event text, we can still do
that.

  https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=trace

Bjorn