在 2025/1/15 10:41, Steven Rostedt 写道:
On Wed, 15 Jan 2025 09:37:53 +0800
Shuai Xue <[email protected]> wrote:
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
/* The following routines constitute the bulk of the
hotplug controller logic
*/
@@ -244,12 +247,20 @@ void pciehp_handle_presence_or_link_change(struct
controller *ctrl, u32 events)
case ON_STATE:
ctrl->state = POWEROFF_STATE;
mutex_unlock(&ctrl->state_lock);
- if (events & PCI_EXP_SLTSTA_DLLSC)
+ if (events & PCI_EXP_SLTSTA_DLLSC) {
ctrl_info(ctrl, "Slot(%s): Link Down\n",
slot_name(ctrl));
- if (events & PCI_EXP_SLTSTA_PDC)
+ 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);
+ }
+ if (events & PCI_EXP_SLTSTA_PDC) {
ctrl_info(ctrl, "Slot(%s): Card not present\n",
slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_NOT_PRESENT);
+ }
pciehp_disable_slot(ctrl, SURPRISE_REMOVAL);
break;
default:
@@ -269,6 +280,9 @@ void pciehp_handle_presence_or_link_change(struct
controller *ctrl, u32 events)
INDICATOR_NOOP);
ctrl_info(ctrl, "Slot(%s): Card not present\n",
slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_NOT_PRESENT);
}
mutex_unlock(&ctrl->state_lock);
return;
@@ -281,12 +295,19 @@ void pciehp_handle_presence_or_link_change(struct
controller *ctrl, u32 events)
case OFF_STATE:
ctrl->state = POWERON_STATE;
mutex_unlock(&ctrl->state_lock);
- if (present)
+ if (present) {
ctrl_info(ctrl, "Slot(%s): Card present\n",
slot_name(ctrl));
- if (link_active)
- ctrl_info(ctrl, "Slot(%s): Link Up\n",
- slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_PRESENT);
+ }
+ if (link_active) {
+ ctrl_info(ctrl, "Slot(%s): Link Up\n", slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_LINK_UP);
+ }
ctrl->request_result = pciehp_enable_slot(ctrl);
break;
default:
diff --git a/drivers/pci/hotplug/trace.h b/drivers/pci/hotplug/trace.h
new file mode 100644
index 000000000000..1415ac505cb5
--- /dev/null
+++ b/drivers/pci/hotplug/trace.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#if !defined(_TRACE_HW_EVENT_PCI_HP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HW_EVENT_PCI_HP_H
+
+#include <linux/tracepoint.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pci
+
+TRACE_EVENT(pci_hp_event,
+
+ TP_PROTO(const char *port_name,
+ const char *slot,
+ const int event),
+
+ TP_ARGS(port_name, slot, event),
+
+ TP_STRUCT__entry(
+ __string( port_name, port_name )
+ __string( slot, slot )
+ __field( int, event )
Then the above would be:
TP_PROTO(struct controller *ctrl, int event),
// don't really need a const int there
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(). For
example:
// drivers/pci/hotplug/pciehp.h
static inline const char *slot_name(struct controller *ctrl)
{
return hotplug_slot_name(&ctrl->hotplug_slot);
}
// drivers/pci/hotplug/shpchp.h
static inline const char *slot_name(struct slot *slot)
{
return hotplug_slot_name(&slot->hotplug_slot);
}
So, IMHO, pass port_name and slot_name from each driver is more simple.
+ @Lukas for hotplug part.
-- Steve
Thanks.
Best Regards,
Shuai
