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

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

在 2025/9/23 15:34, Ilpo Järvinen 写道:
> On Tue, 23 Sep 2025, Ilpo Järvinen wrote:
> 
>> On Tue, 23 Sep 2025, Shuai Xue wrote:
>>
>>>
>>>
>>> 在 2025/9/23 14:46, Ilpo Järvinen 写道:
>>>> On Tue, 23 Sep 2025, Shuai Xue wrote:
>>>>
>>>>>
>>>>>
>>>>> 在 2025/9/22 21:10, Ilpo Järvinen 写道:
>>>>>> On Sat, 20 Sep 2025, 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 following output is generated when a device is hotplugged:
>>>>>>>
>>>>>>> $ echo 1 > /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
>>>>>>> $ cat /sys/kernel/debug/tracing/trace_pipe
>>>>>>>       irq/51-pciehp-88      [001] .....  1311.177459: pci_hp_event:
>>>>>>> 0000:00:02.0 slot:10, event:CARD_PRESENT
>>>>>>>
>>>>>>>       irq/51-pciehp-88      [001] .....  1311.177566: pci_hp_event:
>>>>>>> 0000:00:02.0 slot:10, event:LINK_UP
>>>>>>>
>>>>>>> Suggested-by: Lukas Wunner <lukas@wunner.de>
>>>>>>> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
>>>>>>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>>>>>>> Reviewed-by: Lukas Wunner <lukas@wunner.de>
>>>>>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>>>>> ---
>>>>>>>     drivers/pci/Makefile              |  2 +
>>>>>>>     drivers/pci/hotplug/Makefile      |  3 +-
>>>>>>>     drivers/pci/hotplug/pciehp_ctrl.c | 31 ++++++++++++---
>>>>>>>     drivers/pci/trace.c               | 11 ++++++
>>>>>>>     include/trace/events/pci.h        | 63
>>>>>>> +++++++++++++++++++++++++++++++
>>>>>>>     include/uapi/linux/pci.h          |  7 ++++
>>>>>>>     6 files changed, 110 insertions(+), 7 deletions(-)
>>>>>>>     create mode 100644 drivers/pci/trace.c
>>>>>>>     create mode 100644 include/trace/events/pci.h
>>>>>>>
>>>>>>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>>>>>> index 67647f1880fb..bf389bc4dd3c 100644
>>>>>>> --- a/drivers/pci/Makefile
>>>>>>> +++ b/drivers/pci/Makefile
>>>>>>> @@ -45,3 +45,5 @@ obj-y				+= controller/
>>>>>>>     obj-y				+= switch/
>>>>>>>       subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
>>>>>>> +
>>>>>>> +CFLAGS_trace.o := -I$(src)
>>>>>>> diff --git a/drivers/pci/hotplug/Makefile
>>>>>>> b/drivers/pci/hotplug/Makefile
>>>>>>> index 40aaf31fe338..d41f7050b072 100644
>>>>>>> --- a/drivers/pci/hotplug/Makefile
>>>>>>> +++ b/drivers/pci/hotplug/Makefile
>>>>>>> @@ -65,7 +65,8 @@ rpadlpar_io-objs	:=	rpadlpar_core.o \
>>>>>>>     pciehp-objs		:=	pciehp_core.o	\
>>>>>>>     				pciehp_ctrl.o	\
>>>>>>>     				pciehp_pci.o	\
>>>>>>> -				pciehp_hpc.o
>>>>>>> +				pciehp_hpc.o	\
>>>>>>> +				../trace.o
>>>>>>
>>>>>> To make it useful for any PCI tracing, not juse hotplug, this object
>>>>>> file
>>>>>> should be added in drivers/pci/Makefile, not here.
>>>>>
>>>>> Make sence. How about adding to the main CONFIG_PCI object:
>>>>>
>>>>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>>>> index bf389bc4dd3c..d7f83d06351d 100644
>>>>> --- a/drivers/pci/Makefile
>>>>> +++ b/drivers/pci/Makefile
>>>>> @@ -5,7 +5,7 @@
>>>>>    obj-$(CONFIG_PCI)              += access.o bus.o probe.o host-bridge.o \
>>>>>                                      remove.o pci.o pci-driver.o search.o \
>>>>>                                      rom.o setup-res.o irq.o vpd.o \
>>>>> -                                  setup-bus.o vc.o mmap.o devres.o
>>>>> +                                  setup-bus.o vc.o mmap.o devres.o
>>>>> trace.o
>>>>>
>>>>>    obj-$(CONFIG_PCI)              += msi/
>>>>>    obj-$(CONFIG_PCI)              += pcie/
>>>>
>>>> Yes, that's the right place to add it.
>>>>
>>>
>>> Thanks for confirm.
>>> Will send a new version to fix it.
>>
>> I actually now started to wonder if it should be made depend on some
>> tracing related config (sending this out quickly if you were just
>> waiting for my confirmation to send quickly... I'm still investigating
>> what other subsystems do).
> 
> Probably this is what we actually want:
> 
> obj-$(CONFIG_TRACING)			+= trace.o
> 


Thanks for the input, lots of trace.o, e.g. for cxl and nvme, are compiled
under CONFIG_TRACING.

Will use CONFIG_TRACING :)

Thanks.
Shuai