[PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Prarit Bhargava]

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

I didn't see any ACKs on this so I'm resubmitting....

I discovered an issue where a hwif_init failure lead to /proc/ide files 
being created for devices that failed probes.  This resulted in 
oops/WARN_ON/BUG_ON executions through the kernel depending on what 
actions were on going.

The first part of the fix (submitted to linux-ia64 and acpi-devel for 
review -- I'm including it for completeness) was to fix incorrect error 
handling in the ACPI layer:

===== 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-14 08:14:34 -05:00
@@ -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;

       /*

The second part of the fix is to the core IDE layer and to the SGI IOC4 
IDE driver which need to handle errors properly from failed hwif 
initializations.  This patch is attached for review.

[-- Attachment #2: acpi-ide.patch --]
[-- Type: text/plain, Size: 895 bytes --]

===== 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 08:18:43 -05:00
@@ -841,7 +841,10 @@
 	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 =====
--- 1.22/drivers/ide/pci/sgiioc4.c	2005-01-06 20:35:35 -05:00
+++ edited/drivers/ide/pci/sgiioc4.c	2005-01-14 08:18:56 -05:00
@@ -669,7 +669,10 @@
 		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(); 

Re: [PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Bartlomiej Zolnierkiewicz]

On Wed, 26 Jan 2005 08:25:04 -0500, Prarit Bhargava <prarit@sgi.com> wrote:
> I didn't see any ACKs on this so I'm resubmitting....

sorry for the delay

> I discovered an issue where a hwif_init failure lead to /proc/ide files
> being created for devices that failed probes.  This resulted in

create_proc_ide_interfaces() doesn't create /proc/ide files for
hwif->present == 0 interfaces but hwif_init() sets hwif->present
to 0 too late.  Also this hwif->present games are silly and should
be removed after your patch is applied and locking is verified.

> The second part of the fix is to the core IDE layer and to the SGI IOC4
> IDE driver which need to handle errors properly from failed hwif
> initializations.  This patch is attached for review.

please remember about Signed-off-by:

> ===== 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 08:18:43 -05:00
> @@ -841,7 +841,10 @@
>         if (fixup)
>                 fixup(hwif);
> 
> -       hwif_init(hwif);
> +       if (!hwif_init(hwif)) {
> +               printk("%s: Failed to initialize IDE interface\n", hwif->name);
> +               return -1;
> +       }

Could you fix hwif_init() to return meaningful error codes
and return error from hwif_init() instead of '-1'?

>         if (hwif->present) {
>                 u16 unit = 0;
> ===== 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 08:18:56 -05:00
> @@ -669,7 +669,10 @@
>                 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);

redundant, error message is printed in probe_hwif_init()

> +               return -EIO;

please fix probe_hwif_init() to return error codes

> +       }
> 
>         /* Create /proc/ide entries */
>         create_proc_ide_interfaces();

I removed redundant printk and applied the patch

Re: [PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Prarit Bhargava]

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

Thanks for the comments Bartlomiej -- your input is always appreciated.

Here's a new patch taking into account your comments.  I have updated 
hwif_init
to return 0 on success and -errno on failure.

Thanks again,

P.

[-- Attachment #2: acpi-ide2.patch --]
[-- Type: text/plain, Size: 1942 bytes --]

Signed-off-by: Prarit Bhargava <prarit@sgi.com>

===== ide-probe.c 1.90 vs edited =====
--- 1.90/drivers/ide/ide-probe.c	2004-12-10 14:12:14 -05:00
+++ edited/ide-probe.c	2005-02-08 08:22:37 -05:00
@@ -841,7 +841,10 @@
 	if (fixup)
 		fixup(hwif);
 
-	hwif_init(hwif);
+	if (hwif_init(hwif) < 0) {
+		printk("%s: Failed to initialize IDE interface\n", hwif->name);
+		return -1;
+	}
 
 	if (hwif->present) {
 		u16 unit = 0;
@@ -1245,20 +1248,22 @@
 	int old_irq, unit;
 
 	if (!hwif->present)
-		return 0;
+		return -ENODEV;
 
 	if (!hwif->irq) {
 		if (!(hwif->irq = ide_default_irq(hwif->io_ports[IDE_DATA_OFFSET])))
 		{
 			printk("%s: DISABLED, NO IRQ\n", hwif->name);
-			return (hwif->present = 0);
+			hwif->present = 0;
+			return -EIO;
 		}
 	}
 #ifdef CONFIG_BLK_DEV_HD
 	if (hwif->irq == HD_IRQ && hwif->io_ports[IDE_DATA_OFFSET] != HD_DATA) {
 		printk("%s: CANNOT SHARE IRQ WITH OLD "
 			"HARDDISK DRIVER (hd.c)\n", hwif->name);
-		return (hwif->present = 0);
+		hwif->present = 0;
+		return -EIO;
 	}
 #endif /* CONFIG_BLK_DEV_HD */
 
@@ -1266,7 +1271,7 @@
 	hwif->present = 0;
 
 	if (register_blkdev(hwif->major, hwif->name))
-		return 0;
+		return -EIO;
 
 	if (!hwif->sg_max_nents)
 		hwif->sg_max_nents = PRD_ENTRIES;
@@ -1305,7 +1310,7 @@
 done:
 	init_gendisk(hwif);
 	hwif->present = 1;	/* success */
-	return 1;
+	return 0;
 
 out_disks:
 	for (unit = 0; unit < MAX_DRIVES; unit++) {
@@ -1315,7 +1320,7 @@
 	}
 out:
 	unregister_blkdev(hwif->major, hwif->name);
-	return 0;
+	return -EIO;
 }
 
 int ideprobe_init (void)

===== sgiioc4.c 1.22 vs edited =====
--- 1.22/drivers/ide/pci/sgiioc4.c	2005-01-06 20:35:35 -05:00
+++ edited/sgiioc4.c	2005-02-08 08:54:27 -05:00
@@ -669,7 +669,8 @@
 		printk(KERN_INFO "%s: %s Bus-Master DMA disabled\n",
 		       hwif->name, d->name);
 
-	probe_hwif_init(hwif);
+	if (probe_hwif_init(hwif))
+		return -EIO;
 
 	/* Create /proc/ide entries */
 	create_proc_ide_interfaces(); 

Re: [PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Prarit Bhargava]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]

Bartlomiej pointed out that the sgiioc4.c changes have made their way 
into Linus' tree.

Here's an updated change set.

Thanks,

P.

[-- Attachment #2: acpi-ide3.patch --]
[-- Type: text/plain, Size: 1546 bytes --]

Signed-off-by: Prarit Bhargava <prarit@sgi.com>

===== ide-probe.c 1.90 vs edited =====
--- 1.90/drivers/ide/ide-probe.c	2004-12-10 14:12:14 -05:00
+++ edited/ide-probe.c	2005-02-08 10:41:46 -05:00
@@ -841,7 +841,10 @@
 	if (fixup)
 		fixup(hwif);
 
-	hwif_init(hwif);
+	if (hwif_init(hwif) < 0) {
+		printk("%s: Failed to initialize IDE interface\n", hwif->name);
+		return -1;
+	}
 
 	if (hwif->present) {
 		u16 unit = 0;
@@ -1245,20 +1248,22 @@
 	int old_irq, unit;
 
 	if (!hwif->present)
-		return 0;
+		return -ENODEV;
 
 	if (!hwif->irq) {
 		if (!(hwif->irq = ide_default_irq(hwif->io_ports[IDE_DATA_OFFSET])))
 		{
 			printk("%s: DISABLED, NO IRQ\n", hwif->name);
-			return (hwif->present = 0);
+			hwif->present = 0;
+			return -EIO;
 		}
 	}
 #ifdef CONFIG_BLK_DEV_HD
 	if (hwif->irq == HD_IRQ && hwif->io_ports[IDE_DATA_OFFSET] != HD_DATA) {
 		printk("%s: CANNOT SHARE IRQ WITH OLD "
 			"HARDDISK DRIVER (hd.c)\n", hwif->name);
-		return (hwif->present = 0);
+		hwif->present = 0;
+		return -EIO;
 	}
 #endif /* CONFIG_BLK_DEV_HD */
 
@@ -1266,7 +1271,7 @@
 	hwif->present = 0;
 
 	if (register_blkdev(hwif->major, hwif->name))
-		return 0;
+		return -EIO;
 
 	if (!hwif->sg_max_nents)
 		hwif->sg_max_nents = PRD_ENTRIES;
@@ -1305,7 +1310,7 @@
 done:
 	init_gendisk(hwif);
 	hwif->present = 1;	/* success */
-	return 1;
+	return 0;
 
 out_disks:
 	for (unit = 0; unit < MAX_DRIVES; unit++) {
@@ -1315,7 +1320,7 @@
 	}
 out:
 	unregister_blkdev(hwif->major, hwif->name);
-	return 0;
+	return -EIO;
 }
 
 int ideprobe_init (void)

Re: [PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Bartlomiej Zolnierkiewicz]

On Tue, 08 Feb 2005 10:53:56 -0500, Prarit Bhargava <prarit@sgi.com> wrote:
> Bartlomiej pointed out that the sgiioc4.c changes have made their way
                                                ^^^^^^^^
all changes

> into Linus' tree.
> 
> Here's an updated change set.
> 
> Thanks,
> 
> P.
> 
> 
> Signed-off-by: Prarit Bhargava <prarit@sgi.com>
> 
> ===== ide-probe.c 1.90 vs edited =====
> --- 1.90/drivers/ide/ide-probe.c        2004-12-10 14:12:14 -05:00
> +++ edited/ide-probe.c  2005-02-08 10:41:46 -05:00
> @@ -841,7 +841,10 @@
>         if (fixup)
>                 fixup(hwif);
> 
> -       hwif_init(hwif);
> +       if (hwif_init(hwif) < 0) {
> +               printk("%s: Failed to initialize IDE interface\n", hwif->name);
> +               return -1;
> +       }
> 
>         if (hwif->present) {
>                 u16 unit = 0;

this one chunk was in merged patch already

> @@ -1245,20 +1248,22 @@
>         int old_irq, unit;
> 
>         if (!hwif->present)
> -               return 0;
> +               return -ENODEV;

this is wrong, there is return 1 (not 0) now because of fix needed
for the previous patch (error message for interface with no drives)

No problem, I can hand-merge this patch but these changes
need some description, "fix hwif_init() return codes" perhaps?

Re: [PATCH]: [RFC] Fix error handlnig in hwif-init and sgiioc4 driver

[Prarit Bhargava]

Bartlomiej,

I did a fresh check out of linux-2.6 and diff'd?  That's strange ...

Anyhow, how about "Fixes to IDE hwif initialization error handling".

IMO it better captures what I'm trying to fix :)

P.


>
>No problem, I can hand-merge this patch but these changes
>need some description, "fix hwif_init() return codes" perhaps?
>
>  
>