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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4417 bytes --]

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).

-- 
 i.