Skip to contentSkip to conversation

acpi: fix single linked list manipulation

PATCH In review Akinobu Mita 2 participants 4 messages Oct 28, 2006 – Oct 29, 2006

Subsystems (2)
ACPI PCI
lore
Share & tools
lore

Akinobu Mita

author
#0 lore raw

PATCH 1/1 acpi: fix single linked list manipulation

This patch fixes single linked list manipulation for sub_driver. If the remving entry is not on the head of the sub_driver list, it goes into infinate loop.

Though that infinate loop doesn't happen. Because the only user of acpi_pci_register_dirver() is acpiphp.

Cc Len Brown <[email protected]>
Signed-off-by Akinobu Mita <[email protected]>
 drivers/acpi/pci_root.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/acpi/pci_root.c +5 -4
Index: work-fault-inject/drivers/acpi/pci_root.c
===================================================================
@@ -98,11 +98,12 @@ void acpi_pci_unregister_driver(struct a
 
 	struct acpi_pci_driver **pptr = &sub_driver;
 	while (*pptr) {
-		if (*pptr != driver)
-			continue;
-		*pptr = (*pptr)->next;
-		break;
+		if (*pptr == driver)
+			break;
+		pptr = &(*pptr)->next;
 	}
+	BUG_ON(!*pptr);
+	*pptr = (*pptr)->next;
 
 	if (!driver->remove)
 		return;
#1 lore raw
Quoted 6 lines · Akinobu Mita

This patch fixes single linked list manipulation for sub_driver. If the remving entry is not on the head of the sub_driver list, it goes into infinate loop.

Though that infinate loop doesn't happen. Because the only user of acpi_pci_register_dirver() is acpiphp.

Any chance to just switch the driver to use the list.h APIs instead of opencoding lists?

author
#2 lore raw

PATCH 1/1 acpi: use list.h API for sub_driver list

Quoted 2 lines · Christoph Hellwig

Any chance to just switch the driver to use the list.h APIs instead of opencoding lists?

Subject: [PATCH -mm] acpi: use list.h API for sub_driver list

Use the list.h APIs instead of opencoding lists.

Cc Len Brown <[email protected]>
Signed-off-by Akinobu Mita <[email protected]>
drivers/acpi/pci_root.c +3 -13
Index: work-fault-inject/drivers/acpi/pci_root.c
===================================================================
@@ -65,17 +65,14 @@ struct acpi_pci_root {
 
 static LIST_HEAD(acpi_pci_roots);
 
-static struct acpi_pci_driver *sub_driver;
+static LIST_HEAD(sub_driver);
 
 int acpi_pci_register_driver(struct acpi_pci_driver *driver)
 {
 	int n = 0;
 	struct list_head *entry;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr)
-		pptr = &(*pptr)->next;
-	*pptr = driver;
+	list_add_tail(&driver->list, &sub_driver);
 
 	if (!driver->add)
 		return 0;

@@ -96,14 +93,7 @@ void acpi_pci_unregister_driver(struct a
 {
 	struct list_head *entry;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr) {
-		if (*pptr == driver)
-			break;
-		pptr = &(*pptr)->next;
-	}
-	BUG_ON(!*pptr);
-	*pptr = (*pptr)->next;
+	list_del(&driver->list);
 
 	if (!driver->remove)
 		return;
include/linux/acpi.h +1 -1
Index: work-fault-inject/include/linux/acpi.h
===================================================================
@@ -480,7 +480,7 @@ void acpi_penalize_isa_irq(int irq, int 
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 struct acpi_pci_driver {
-	struct acpi_pci_driver *next;
+	struct list_head list;
 	int (*add)(acpi_handle handle);
 	void (*remove)(acpi_handle handle);
 };
#3 lore raw

Re: [PATCH -mm] acpi: use list.h API for sub_driver list

Quoted 7 lines · Akinobu Mita
Quoted 2 lines · Christoph Hellwig

Any chance to just switch the driver to use the list.h APIs instead of opencoding lists?

Subject: [PATCH -mm] acpi: use list.h API for sub_driver list

Use the list.h APIs instead of opencoding lists.

Maybe it's just me but I don't see a place where we actually ever iterate this list. Len, do you plan to introduce users of the list anytime soon? In that case the patch below looks good to me, else we should just rip it out completely.

Quoted 48 lines
Cc Len Brown <[email protected]>
Signed-off-by Akinobu Mita <[email protected]>

Index: work-fault-inject/drivers/acpi/pci_root.c
===================================================================

drivers/acpi/pci_root.c +3 -13
@@ -65,17 +65,14 @@ struct acpi_pci_root {
 
 static LIST_HEAD(acpi_pci_roots);
 
-static struct acpi_pci_driver *sub_driver;
+static LIST_HEAD(sub_driver);
 
 int acpi_pci_register_driver(struct acpi_pci_driver *driver)
 {
 	int n = 0;
 	struct list_head *entry;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr)
-		pptr = &(*pptr)->next;
-	*pptr = driver;
+	list_add_tail(&driver->list, &sub_driver);
 
 	if (!driver->add)
 		return 0;

@@ -96,14 +93,7 @@ void acpi_pci_unregister_driver(struct a
 {
 	struct list_head *entry;
 
-	struct acpi_pci_driver **pptr = &sub_driver;
-	while (*pptr) {
-		if (*pptr == driver)
-			break;
-		pptr = &(*pptr)->next;
-	}
-	BUG_ON(!*pptr);
-	*pptr = (*pptr)->next;
+	list_del(&driver->list);
 
 	if (!driver->remove)
 		return;

Index: work-fault-inject/include/linux/acpi.h
===================================================================

include/linux/acpi.h void acpi_penalize_isa_irq(int irq, int · line 486

Quoted 8 lines
include/linux/acpi.h +1 -1
@@ -480,7 +480,7 @@ void acpi_penalize_isa_irq(int irq, int 
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 struct acpi_pci_driver {
-	struct acpi_pci_driver *next;
+	struct list_head list;
 	int (*add)(acpi_handle handle);
 	void (*remove)(acpi_handle handle);
 };

---end quoted text---