Skip to contentSkip to conversation

: Error in ACPI error handling results in WARN_ON

PATCH In review Prarit Bhargava 2 participants 3 messages Jan 14, 2005

Subsystems (2)
ACPI PCI
lore
Share & tools
lore

Prarit Bhargava

author
#0 lore raw

PATCH 1/1 : Error in ACPI error handling results in WARN_ON

The issue here is the error handling within the ACPI layer. gsi = 0 is a valid return value from acpi_pci_irq_lookup, while gsi = (-1) is the value returned when a PRT cannot be found.

This error in error handling can result in a WARN_ON at arch/ia64/kernel/iosapic.c:622 .

The remaining fix is to complete error handling within the core IDE layer and the IOC4 IDE driver.

===== drivers/acpi/pci_irq.c 1.35 vs edited =====

drivers/acpi/pci_irq.c +2 -2
@@ -487,10 +487,10 @@
 	 * If no PRT entry was found, we'll try to derive an IRQ from the
 	 * device's parent bridge.
 	 */
-	if (!gsi)
+	if (gsi == (-1))
  		gsi = acpi_pci_irq_derive(dev, pin,
 					  &edge_level, &active_high_low);
-	if (!gsi)
+	if (!gsi == (-1))
 		return_VOID;
 
 	/*

===== drivers/ide/ide-probe.c 1.90 vs edited =====

drivers/ide/ide-probe.c +5 -1
@@ -841,7 +841,11 @@
 	if (fixup)
 		fixup(hwif);
 
-	hwif_init(hwif);
+	if (!hwif_init(hwif))
+	{
+		printk("%s: Failed to initialize IDE interface\n", hwif->name);
+		return -1;
+	}
 
 	if (hwif->present) {
 		u16 unit = 0;

===== drivers/ide/pci/sgiioc4.c 1.22 vs edited =====

drivers/ide/pci/sgiioc4.c +5 -1
@@ -669,7 +669,11 @@
 		printk(KERN_INFO "%s: %s Bus-Master DMA disabled\n",
 		       hwif->name, d->name);
 
-	probe_hwif_init(hwif);
+	if (probe_hwif_init(hwif))
+	{
+		printk(KERN_INFO "%s: initialization failed\n", hwif->name);
+		return -EIO;
+	}
 
 	/* Create /proc/ide entries */
 	create_proc_ide_interfaces();
#1 lore raw

drivers/acpi/pci_irq.c line 490

Quoted 12 lines · Prarit Bhargava

The issue here is the error handling within the ACPI layer. gsi = 0 is a valid return value from acpi_pci_irq_lookup, while gsi = (-1) is the value returned when a PRT cannot be found.

=== drivers/acpi/pci_irq.c 1.35 vs edited ==> --- 1.35/drivers/acpi/pci_irq.c 2005-01-04 21:48:17 -05:00
+++ edited/drivers/acpi/pci_irq.c 2005-01-13 16:08:10 -05:00

drivers/acpi/pci_irq.c +1 -1
@@ -487,10 +487,10 @@
 	 * If no PRT entry was found, we'll try to derive an IRQ from the
 	 * device's parent bridge.
 	 */
-	if (!gsi)
+	if (gsi = (-1))

Brackets here just look clumsy; better to write:

	if (gsi = -1)

drivers/acpi/pci_irq.c

Quoted 4 lines
drivers/acpi/pci_irq.c +1 -1
  		gsi = acpi_pci_irq_derive(dev, pin,
 					  &edge_level, &active_high_low);
-	if (!gsi)
+	if (!gsi = (-1))

And this is a mistake -- you didn't remove the ! Again, it should be:

	if (gsi = -1)
Quoted 5 lines

=== drivers/ide/ide-probe.c 1.90 vs edited ==> --- 1.90/drivers/ide/ide-probe.c 2004-12-10 14:12:14 -05:00
+++ edited/drivers/ide/ide-probe.c 2005-01-14 07:16:42 -05:00

drivers/ide/ide-probe.c +0 -0
@@ -841,7 +841,11 @@
 	if (fixup)
 		fixup(hwif);

drivers/ide/ide-probe.c line 851

Quoted 10 lines
drivers/ide/ide-probe.c +5 -1
 
-	hwif_init(hwif);
+	if (!hwif_init(hwif))
+	{
+		printk("%s: Failed to initialize IDE interface\n", hwif->name);
+		return -1;
+	}
 
 	if (hwif->present) {
 		u16 unit = 0;

Please send this one to [email protected]

Quoted 5 lines

=== drivers/ide/pci/sgiioc4.c 1.22 vs edited ==> --- 1.22/drivers/ide/pci/sgiioc4.c 2005-01-06 20:35:35 -05:00
+++ edited/drivers/ide/pci/sgiioc4.c 2005-01-14 07:23:27 -05:00

drivers/ide/pci/sgiioc4.c +0 -0
@@ -669,7 +669,11 @@
 		printk(KERN_INFO "%s: %s Bus-Master DMA disabled\n",
 		       hwif->name, d->name);

drivers/ide/pci/sgiioc4.c line 679

Quoted 10 lines
drivers/ide/pci/sgiioc4.c +5 -1
 
-	probe_hwif_init(hwif);
+	if (probe_hwif_init(hwif))
+	{
+		printk(KERN_INFO "%s: initialization failed\n", hwif->name);
+		return -EIO;
+	}
 
 	/* Create /proc/ide entries */
 	create_proc_ide_interfaces();

May as well send this one along with it. BTW, the normal coding style is to place brackets on the same line as the conditional:

	if (probe_hwif_init(hwif)) {
		printk(...)
		return -EIO;
	}
Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
author
#2 lore raw
In reply to Prarit Bhargava

Thanks for your input Matthew -- it's always appreciated.

Here's a new patch taking into account Matthew's comments.

===== drivers/acpi/pci_irq.c 1.35 vs edited =====

drivers/acpi/pci_irq.c +2 -2
@@ -487,10 +487,10 @@
 	 * If no PRT entry was found, we'll try to derive an IRQ from the
 	 * device's parent bridge.
 	 */
-	if (!gsi)
+	if (gsi == -1)
  		gsi = acpi_pci_irq_derive(dev, pin,
 					  &edge_level, &active_high_low);
-	if (!gsi)
+	if (gsi == -1)
 		return_VOID;
 
 	/*