| Message ID | 20180511163923.22116-1-bp@alien8.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Fri, May 11, 2018 at 06:39:23PM +0200, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > Just move the actual function up so that it is visible to its user > aer_recover_queue(). Good idea. Not *directly* related, but I'm really tired of the clutter of all these separate files, and I'm thinking of squashing drivers/pci/pcie/aer/* => drivers/pci/pcie/aer.c drivers/pci/pcie/portdrv* => drivers/pci/pcie/portdrv.c drivers/pci/hotplug/pciehp* => drivers/pci/hotplug/pciehp.c drivers/pci/hotplug/shpchp* => drivers/pci/hotplug/shpchp.c etc. That would let us make many more things static and (more importantly, IMO), make it easier to browse the code. > No functional changes. > > Signed-off-by: Borislav Petkov <bp@suse.de> > --- > drivers/pci/pcie/aer/aerdrv_core.c | 45 +++++++++++++++--------------- > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c > index 0ea5acc40323..a9e575189571 100644 > --- a/drivers/pci/pcie/aer/aerdrv_core.c > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > @@ -567,8 +567,6 @@ static void handle_error_source(struct pcie_device *aerdev, > } > > #ifdef CONFIG_ACPI_APEI_PCIEAER > -static void aer_recover_work_func(struct work_struct *work); > - > #define AER_RECOVER_RING_ORDER 4 > #define AER_RECOVER_RING_SIZE (1 << AER_RECOVER_RING_ORDER) > > @@ -582,6 +580,28 @@ struct aer_recover_entry { > > static DEFINE_KFIFO(aer_recover_ring, struct aer_recover_entry, > AER_RECOVER_RING_SIZE); > + > +static void aer_recover_work_func(struct work_struct *work) > +{ > + struct aer_recover_entry entry; > + struct pci_dev *pdev; > + > + while (kfifo_get(&aer_recover_ring, &entry)) { > + pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, > + entry.devfn); > + if (!pdev) { > + pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", > + entry.domain, entry.bus, > + PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); > + continue; > + } > + cper_print_aer(pdev, entry.severity, entry.regs); > + if (entry.severity != AER_CORRECTABLE) > + do_recovery(pdev, entry.severity); > + pci_dev_put(pdev); > + } > +} > + > /* > * Mutual exclusion for writers of aer_recover_ring, reader side don't > * need lock, because there is only one reader and lock is not needed > @@ -611,27 +631,6 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn, > spin_unlock_irqrestore(&aer_recover_ring_lock, flags); > } > EXPORT_SYMBOL_GPL(aer_recover_queue); > - > -static void aer_recover_work_func(struct work_struct *work) > -{ > - struct aer_recover_entry entry; > - struct pci_dev *pdev; > - > - while (kfifo_get(&aer_recover_ring, &entry)) { > - pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, > - entry.devfn); > - if (!pdev) { > - pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", > - entry.domain, entry.bus, > - PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); > - continue; > - } > - cper_print_aer(pdev, entry.severity, entry.regs); > - if (entry.severity != AER_CORRECTABLE) > - do_recovery(pdev, entry.severity); > - pci_dev_put(pdev); > - } > -} > #endif > > /** > -- > 2.17.0.391.g1f1cddd558b5 >
On Fri, May 11, 2018 at 02:00:13PM -0500, Bjorn Helgaas wrote: > On Fri, May 11, 2018 at 06:39:23PM +0200, Borislav Petkov wrote: > > From: Borislav Petkov <bp@suse.de> > > > > Just move the actual function up so that it is visible to its user > > aer_recover_queue(). > > Good idea. > > Not *directly* related, but I'm really tired of the clutter of all these > separate files, and I'm thinking of squashing > > drivers/pci/pcie/aer/* => drivers/pci/pcie/aer.c > drivers/pci/pcie/portdrv* => drivers/pci/pcie/portdrv.c > drivers/pci/hotplug/pciehp* => drivers/pci/hotplug/pciehp.c > drivers/pci/hotplug/shpchp* => drivers/pci/hotplug/shpchp.c > etc. > > That would let us make many more things static and (more importantly, > IMO), make it easier to browse the code. Yeah, it probably is of no use to have separate compilation units like aerdriver-objs := aerdrv_errprint.o aerdrv_core.o aerdrv.o for example, which practically get always built together.
On Fri, May 11, 2018 at 06:39:23PM +0200, Borislav Petkov wrote: > From: Borislav Petkov <bp@suse.de> > > Just move the actual function up so that it is visible to its user > aer_recover_queue(). > > No functional changes. > > Signed-off-by: Borislav Petkov <bp@suse.de> Applied to pci/aer for v4.18, thanks! > --- > drivers/pci/pcie/aer/aerdrv_core.c | 45 +++++++++++++++--------------- > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c > index 0ea5acc40323..a9e575189571 100644 > --- a/drivers/pci/pcie/aer/aerdrv_core.c > +++ b/drivers/pci/pcie/aer/aerdrv_core.c > @@ -567,8 +567,6 @@ static void handle_error_source(struct pcie_device *aerdev, > } > > #ifdef CONFIG_ACPI_APEI_PCIEAER > -static void aer_recover_work_func(struct work_struct *work); > - > #define AER_RECOVER_RING_ORDER 4 > #define AER_RECOVER_RING_SIZE (1 << AER_RECOVER_RING_ORDER) > > @@ -582,6 +580,28 @@ struct aer_recover_entry { > > static DEFINE_KFIFO(aer_recover_ring, struct aer_recover_entry, > AER_RECOVER_RING_SIZE); > + > +static void aer_recover_work_func(struct work_struct *work) > +{ > + struct aer_recover_entry entry; > + struct pci_dev *pdev; > + > + while (kfifo_get(&aer_recover_ring, &entry)) { > + pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, > + entry.devfn); > + if (!pdev) { > + pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", > + entry.domain, entry.bus, > + PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); > + continue; > + } > + cper_print_aer(pdev, entry.severity, entry.regs); > + if (entry.severity != AER_CORRECTABLE) > + do_recovery(pdev, entry.severity); > + pci_dev_put(pdev); > + } > +} > + > /* > * Mutual exclusion for writers of aer_recover_ring, reader side don't > * need lock, because there is only one reader and lock is not needed > @@ -611,27 +631,6 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn, > spin_unlock_irqrestore(&aer_recover_ring_lock, flags); > } > EXPORT_SYMBOL_GPL(aer_recover_queue); > - > -static void aer_recover_work_func(struct work_struct *work) > -{ > - struct aer_recover_entry entry; > - struct pci_dev *pdev; > - > - while (kfifo_get(&aer_recover_ring, &entry)) { > - pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, > - entry.devfn); > - if (!pdev) { > - pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", > - entry.domain, entry.bus, > - PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); > - continue; > - } > - cper_print_aer(pdev, entry.severity, entry.regs); > - if (entry.severity != AER_CORRECTABLE) > - do_recovery(pdev, entry.severity); > - pci_dev_put(pdev); > - } > -} > #endif > > /** > -- > 2.17.0.391.g1f1cddd558b5 >
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index 0ea5acc40323..a9e575189571 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -567,8 +567,6 @@ static void handle_error_source(struct pcie_device *aerdev, } #ifdef CONFIG_ACPI_APEI_PCIEAER -static void aer_recover_work_func(struct work_struct *work); - #define AER_RECOVER_RING_ORDER 4 #define AER_RECOVER_RING_SIZE (1 << AER_RECOVER_RING_ORDER) @@ -582,6 +580,28 @@ struct aer_recover_entry { static DEFINE_KFIFO(aer_recover_ring, struct aer_recover_entry, AER_RECOVER_RING_SIZE); + +static void aer_recover_work_func(struct work_struct *work) +{ + struct aer_recover_entry entry; + struct pci_dev *pdev; + + while (kfifo_get(&aer_recover_ring, &entry)) { + pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, + entry.devfn); + if (!pdev) { + pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", + entry.domain, entry.bus, + PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); + continue; + } + cper_print_aer(pdev, entry.severity, entry.regs); + if (entry.severity != AER_CORRECTABLE) + do_recovery(pdev, entry.severity); + pci_dev_put(pdev); + } +} + /* * Mutual exclusion for writers of aer_recover_ring, reader side don't * need lock, because there is only one reader and lock is not needed @@ -611,27 +631,6 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn, spin_unlock_irqrestore(&aer_recover_ring_lock, flags); } EXPORT_SYMBOL_GPL(aer_recover_queue); - -static void aer_recover_work_func(struct work_struct *work) -{ - struct aer_recover_entry entry; - struct pci_dev *pdev; - - while (kfifo_get(&aer_recover_ring, &entry)) { - pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus, - entry.devfn); - if (!pdev) { - pr_err("AER recover: Can not find pci_dev for %04x:%02x:%02x:%x\n", - entry.domain, entry.bus, - PCI_SLOT(entry.devfn), PCI_FUNC(entry.devfn)); - continue; - } - cper_print_aer(pdev, entry.severity, entry.regs); - if (entry.severity != AER_CORRECTABLE) - do_recovery(pdev, entry.severity); - pci_dev_put(pdev); - } -} #endif /**