Enhance dynamic debug facility for PCI subsystem and generally clean up PCI debug mess

Hi,

if you want to get dev_dbg() PCI messages enabled, at least via dynamic debug interface (is there something else?), one has to do:
echo "module probe +p" >/sys/kernel/debug/dynamic_debug/control echo "module pci +p" >/sys/kernel/debug/dynamic_debug/control echo "module slot +p" >/sys/kernel/debug/dynamic_debug/control echo "module quirks +p" >/sys/kernel/debug/dynamic_debug/control
...
(or "file probe.c +p", etc.)
to enable below:

first hits when searching for pci in
/sys/kernel/debug/dynamic_debug/control:
drivers/pci/probe.c:753 [probe]pci_scan_bridge =_ "bus configuration invalid, reconfiguring\012" drivers/pci/probe.c:743 [probe]pci_scan_bridge =_ "scanning [bus %02x-%02x] behind bridge, pass %d\012" drivers/pci/probe.c:1655 [probe]pci_scan_child_bus =_ "bus scan returning with max=%02x\012" drivers/pci/probe.c:1635 [probe]pci_scan_child_bus =_ "fixups for bus\012"
drivers/pci/probe.c:1621 [probe]pci_scan_child_bus =_ "scanning bus\012"
drivers/pci/probe.c:1690 [probe]pci_create_root_bus =_ "bus already known\012"
drivers/pci/pci.c:986 [pci]pci_restore_config_dword =_ "restoring config space at offset %#x (was %#x, writing %#x)\012" drivers/pci/pci.c:1599 [pci]pci_pme_active =_ "PME# %s\012"
drivers/pci/pci.c:2666 [pci]__pci_set_master =_ "%s bus mastering\012"
drivers/pci/pci.c:2796 [pci]pci_set_mwi =_ "enabling Mem-Wr-Inval\012"
drivers/pci/setup-res.c:90 [setup_res]pci_update_resource =_ "BAR %d: set to %pR (PCI address [%#llx-%#llx])\012" drivers/pci/slot.c:107 [slot]pci_slot_release =_ "dev %02x, released physical slot %s\012" drivers/pci/slot.c:309 [slot]pci_create_slot =_ "dev %02x, created physical slot %s\012" drivers/pci/slot.c:359 [slot]pci_destroy_slot =_ "dev %02x, dec refcount to %d\012" drivers/pci/quirks.c:2930 [quirks]pci_do_fixups =_ "calling %pF\012"

My ideas are:
1) Create 3 namespaces: pci, pcie, pci-hotplug
then one can enable dev_dbg messages easier by doing:
echo "module pci +p" >/sys/kernel/debug/dynamic_debug/control echo "module pcie +p" >/sys/kernel/debug/dynamic_debug/control echo "module pci-hotplug +p" >/sys/kernel/debug/dynamic_debug/control if looking at pci hotplug issues as an example.

2) Would it make sense to enhance dynamic debug syntax to be able to pass files, modules, formats,... comma separated (or does there exist something like that already?
Then above can be shortened to:
echo "module pci,pcie,pci-hotplug +p" >/sys/kernel/debug/dynamic_debug/control or passing via boot param:
dyndbg_query="module pci,pcie,pci-hotplug +p"

3) At the end of drivers/pci/Makefile I found:
ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
but I couldn't find anything that could get enabled via defining DEBUG in pci files? It's hard to grep for it, but from what I can see this line and also the whole CONFIG_PCI_DEBUG option can be nuked away without losing anything?

4) Unify PCI hotplug debugging
A special mess are debug messages in PCI hotplug subsystem and modules. There is:
hotplug.debug=1
hotplug.debug_acpi=1
pciehp.pciehp_debug=1
shpchp.shpchp_debug=1
cpqphp.debug=1
and possibly more.
Looks like one introduced his own debug facility with module param, it's own #define dbg(fmt, arg...), etc. and others copied the stuff.
Not sure whether above debug params are protected by a kind of "module params must not get removed just like that, but get deprecated first" rule. I would like to jut convert debug statements to dev_dbg() or pr_debug() ones and simply drop the different x.debug module parameters.
Deprecation time and similar should not be needed for these.

Ah yes, Iirc Jason wanted to enhance dynamic debugging at low level, so that only enabled debugging messages cost runtime performance or similar (x86 only)? Did this work out?

Find below a patch starting with 1.
It's the most intrusive one (renaming drivers/pci/pci.c to drivers/pci/core.c), the rest (introducing pcie and pci-hotplug namespace) should only be Makefile changes in drivers/pci/pcie/ and drivers/pci/hotplug if things work out as expected.

With below patch one would get this /sys/kernel/debug/dynamic_debug/control output for PCI related dynamic debug messages (compare with above, be aware that output is from recent, but slightly different kernel versions):

drivers/pci/probe.c:1699 [pci]pci_create_root_bus =p "bus already known\012"
drivers/pci/probe.c:1673 [pci]pci_scan_child_bus =p "bus scan returning with max=%02x\012" drivers/pci/probe.c:1653 [pci]pci_scan_child_bus =p "fixups for bus\012"
drivers/pci/probe.c:1639 [pci]pci_scan_child_bus =p "scanning bus\012"
drivers/pci/probe.c:728 [pci]pci_scan_bridge =p "scanning [bus %02x-%02x] behind bridge, pass %d\012" drivers/pci/core.c:2890 [pci]pci_set_mwi =p "enabling Mem-Wr-Inval\012"
drivers/pci/core.c:2748 [pci]__pci_set_master =p "%s bus mastering\012"
drivers/pci/core.c:1647 [pci]pci_pme_active =p "PME# %s\012"
drivers/pci/core.c:979 [pci]pci_restore_config_dword =p "restoring config space at offset %#x (was %#x, writing %#x)\012" drivers/pci/setup-res.c:108 [pci]pci_update_resource =p "BAR %d: set to %pR (PCI address [%#llx-%#llx])\012" drivers/pci/slot.c:359 [pci]pci_destroy_slot =p "dev %02x, dec refcount to %d\012" drivers/pci/slot.c:309 [pci]pci_create_slot =p "dev %02x, created physical slot %s\012" drivers/pci/slot.c:107 [pci]pci_slot_release =p "dev %02x, released physical slot %s\012" drivers/pci/quirks.c:2898 [pci]fixup_debug_start =p "calling %pF @ %i for %s\012" drivers/pci/quirks.c:2895 [pci]fixup_debug_start =p "calling %pF\012"
drivers/pci/quirks.c:2916 [pci]fixup_debug_report =p "pci fixup %pF returned after %lld usecs for %s\012"

-------------------
pci: Introduce PCI module namespace for sane dynamic debug handling

This allows to enable PCI dynamic debugging of compiled in drivers/pci/*.o files via:
echo "module pci +p" >/sys/kernel/debug/dynamic_debug/control

Move drivers/pci/pci.c drivers/pci/core.c to avoid name clash.

Index: linux-3.8-rc4-master/drivers/pci/Makefile
===================================================================
@@ -2,73 +2,75 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y		+= access.o bus.o probe.o host-bridge.o remove.o pci.o \
+obj-y		+= pci.o
+
+pci-y		+= access.o bus.o probe.o host-bridge.o remove.o core.o \
 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
 			irq.o vpd.o setup-bus.o
-obj-$(CONFIG_PROC_FS) += proc.o
-obj-$(CONFIG_SYSFS) += slot.o
-obj-$(CONFIG_PCI_GUESTDEV) += guestdev.o
-obj-$(CONFIG_PCI_IOMULTI) += pci-iomul.o
+pci-$(CONFIG_PROC_FS) += proc.o
+pci-$(CONFIG_SYSFS) += slot.o
+pci-$(CONFIG_PCI_GUESTDEV) += guestdev.o
+pci-$(CONFIG_PCI_IOMULTI) += pci-iomul.o
 iomul-$(CONFIG_PCI_IOMULTI) := iomulti.o
-obj-y += $(iomul-y) $(iomul-m)
-obj-$(CONFIG_PCI_RESERVE) += reserve.o
+pci-y += $(iomul-y) $(iomul-m)
+pci-$(CONFIG_PCI_RESERVE) += reserve.o
 
-obj-$(CONFIG_PCI_QUIRKS) += quirks.o
+pci-$(CONFIG_PCI_QUIRKS) += quirks.o
 
 # Build PCI Express stuff if needed
 obj-$(CONFIG_PCIEPORTBUS) += pcie/
 
-obj-$(CONFIG_PCI_IOAPIC) += ioapic.o
+pci-$(CONFIG_PCI_IOAPIC) += ioapic.o
 
 # Build the PCI Hotplug drivers if we were asked to
 obj-$(CONFIG_HOTPLUG_PCI) += hotplug/
 ifdef CONFIG_HOTPLUG_PCI
-obj-y += hotplug-pci.o
+pci-y += hotplug-pci.o
 endif
 
 # Build the PCI MSI interrupt support
-obj-$(CONFIG_PCI_MSI) += msi.o
+pci-$(CONFIG_PCI_MSI) += msi.o
 
 # Build the Hypertransport interrupt support
-obj-$(CONFIG_HT_IRQ) += htirq.o
+pci-$(CONFIG_HT_IRQ) += htirq.o
 
-obj-$(CONFIG_PCI_ATS) += ats.o
-obj-$(CONFIG_PCI_IOV) += iov.o
+pci-$(CONFIG_PCI_ATS) += ats.o
+pci-$(CONFIG_PCI_IOV) += iov.o
 
 #
 # Some architectures use the generic PCI setup functions
 #
-obj-$(CONFIG_X86) += setup-bus.o
-obj-$(CONFIG_ALPHA) += setup-bus.o setup-irq.o
-obj-$(CONFIG_ARM) += setup-bus.o setup-irq.o
-obj-$(CONFIG_UNICORE32) += setup-bus.o setup-irq.o
-obj-$(CONFIG_PARISC) += setup-bus.o
-obj-$(CONFIG_SUPERH) += setup-bus.o setup-irq.o
-obj-$(CONFIG_PPC) += setup-bus.o
-obj-$(CONFIG_FRV) += setup-bus.o
-obj-$(CONFIG_MIPS) += setup-bus.o setup-irq.o
-obj-$(CONFIG_X86_VISWS) += setup-irq.o
-obj-$(CONFIG_MN10300) += setup-bus.o
-obj-$(CONFIG_MICROBLAZE) += setup-bus.o
-obj-$(CONFIG_TILE) += setup-bus.o setup-irq.o
-obj-$(CONFIG_SPARC_LEON) += setup-bus.o setup-irq.o
-obj-$(CONFIG_M68K) += setup-bus.o setup-irq.o
+pci-$(CONFIG_X86) += setup-bus.o
+pci-$(CONFIG_ALPHA) += setup-bus.o setup-irq.o
+pci-$(CONFIG_ARM) += setup-bus.o setup-irq.o
+pci-$(CONFIG_UNICORE32) += setup-bus.o setup-irq.o
+pci-$(CONFIG_PARISC) += setup-bus.o
+pci-$(CONFIG_SUPERH) += setup-bus.o setup-irq.o
+pci-$(CONFIG_PPC) += setup-bus.o
+pci-$(CONFIG_FRV) += setup-bus.o
+pci-$(CONFIG_MIPS) += setup-bus.o setup-irq.o
+pci-$(CONFIG_X86_VISWS) += setup-irq.o
+pci-$(CONFIG_MN10300) += setup-bus.o
+pci-$(CONFIG_MICROBLAZE) += setup-bus.o
+pci-$(CONFIG_TILE) += setup-bus.o setup-irq.o
+pci-$(CONFIG_SPARC_LEON) += setup-bus.o setup-irq.o
+pci-$(CONFIG_M68K) += setup-bus.o setup-irq.o
 
 #
 # ACPI Related PCI FW Functions
 # ACPI _DSM provided firmware instance and string name
 #
-obj-$(CONFIG_ACPI)    += pci-acpi.o
+pci-$(CONFIG_ACPI)    += pci-acpi.o
 
 # SMBIOS provided firmware instance and labels
-obj-$(CONFIG_PCI_LABEL) += pci-label.o
+pci-$(CONFIG_PCI_LABEL) += pci-label.o
 
-obj-$(CONFIG_PCI_SYSCALL) += syscall.o
+pci-$(CONFIG_PCI_SYSCALL) += syscall.o
 
-obj-$(CONFIG_PCI_STUB) += pci-stub.o
+pci-$(CONFIG_PCI_STUB) += pci-stub.o
 
-obj-$(CONFIG_PARAVIRT_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
+pci-$(CONFIG_PARAVIRT_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
 
-obj-$(CONFIG_OF) += of.o
+pci-$(CONFIG_OF) += of.o
 
 ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG

Part of the diff is not shown here. View the raw message.