Skip to contentSkip to conversation

pci/aer: Get rid of aer_recover_work_func() forward declaration

PATCH Applied Borislav Petkov 2 participants 4 messages May 11, 2018 – May 30, 2018 PCI

lore
Share & tools
lore

Borislav Petkov

author
#0 lore raw

PATCH 1/1 pci/aer: Get rid of aer_recover_work_func() forward declaration

From: Borislav Petkov <[email protected]>

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 <[email protected]>
---
 drivers/pci/pcie/aer/aerdrv_core.c | 45 +++++++++++++++---------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/pci/pcie/aer/aerdrv_core.c +22 -23
index 0ea5acc40323..a9e575189571 100644
@@ -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
 
 /**
Signature · 1 line
2.17.0.391.g1f1cddd558b5
#1 lore raw
Quoted 4 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

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.

Quoted 80 lines

No functional changes.

Signed-off-by Borislav Petkov <[email protected]>
---
 drivers/pci/pcie/aer/aerdrv_core.c | 45 +++++++++++++++---------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/pci/pcie/aer/aerdrv_core.c +22 -23
index 0ea5acc40323..a9e575189571 100644
@@ -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
 
 /**
Signature · 1 line
2.17.0.391.g1f1cddd558b5
author
#2 lore raw
Quoted 11 lines · Bjorn Helgaas
Quoted 4 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

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

Quoted 8 lines
  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.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
Applied
#3 lore raw
In reply to Borislav Petkov
Quoted 8 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>

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 <[email protected]>

Applied to pci/aer for v4.18, thanks!

Quoted 77 lines
---
 drivers/pci/pcie/aer/aerdrv_core.c | 45 +++++++++++++++---------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/pci/pcie/aer/aerdrv_core.c +22 -23
index 0ea5acc40323..a9e575189571 100644
@@ -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
 
 /**
Signature · 1 line
2.17.0.391.g1f1cddd558b5