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

[Lukas Wunner <lukas@wunner.de>]

On Wed, Jan 15, 2025 at 11:59:13AM +0800, Shuai Xue wrote:
> 2025/1/15 10:41, Steven Rostedt:
> > On Wed, 15 Jan 2025 09:37:53 +0800 Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> > > +			trace_pci_hp_event(pci_name(ctrl->pcie->port),
> > > +					   slot_name(ctrl),
> > > +					   PCI_HOTPLUG_LINK_DOWN);
> > 
> > Hmm, can't you just pass in the ctrl pointer to the tracepoint?
> > 
> > 			trace_pci_hp_event(ctrl, PCI_HOTPLUG_LINK_DOWN);
> > 
> > Then the above would be:
> > 
> > 	TP_PROTO(struct controller *ctrl, int event),
> > 
> > 	TP_ARGS(ctrl, event),
> > 
> > 	TP_STRUCT__entry(
> > 		__string(	port_name,	pci_name(ctrl->pcie->port)	)
> > 		__string(	slot,		slot_name(ctrl)			)
> > 		__field(	int,		event				)
> > 
> > and everything else could be the same.
> 
> Maybe it's not a good idea.
> 
> I think pci_hp_event is a generic event for pciehp, shpchp, octep_hp, etc.
> But each hotplug driver has different `struct controller` and slot_name().
[...]
> So, IMHO, pass port_name and slot_name from each driver is more simple.
> 
> + @Lukas for hotplug part.

You're right Shuai, there's several hotplug drivers in drivers/pci/hotplug/
and pciehp is just one of them.  It's quite possible that other drivers
besides pciehp will want to add trace points as well.  For consistency,
the trace event definitions need to work for all drivers.

Thanks,

Lukas