[PATCH] ide-disk oopses on boot

[PATCH] ide-disk oopses on boot

[Petr Vandrovec]

Hello Christoph,
  back in June your '[PATCH] NUMA aware block device control structure 
allocation' patch went in, changing ide-disk.c's code:

-   g = alloc_disk(1 << PARTN_BITS);
+   g = alloc_disk_node(1 << PARTN_BITS,
+                       pcibus_to_node(drive->hwif->pci_dev->bus));

  Problem is that pci_dev may be NULL - and it is NULL for example with
kernel I've just built, with amd IDE driver built as a module while with
ide-disk built into the kernel.

  I think that you probably want to guard your code by 
'if (drive->hwif->pci_dev)', as besides my silly configuration mistake 
also ISA devices have a chance to have pci_dev NULL.  Not that there are 
any such users if nobody hit it in the last 6 weeks...

  I've just built amd IDE driver into the kernel, where it belongs anyway,
but just in case please apply this patch...
						Thanks,
							Petr Vandrovec


Signed-off-by: Petr Vandrovec <vandrove@vc.cvut.cz>

--- linux-2.6.13-rc6-00dd.dist/drivers/ide/ide-disk.c	2005-08-09 13:14:26.000000000 +0200
+++ linux-2.6.13-rc6-00dd/drivers/ide/ide-disk.c	2005-08-09 15:11:51.000000000 +0200
@@ -1219,8 +1219,12 @@
 	if (!idkp)
 		goto failed;
 
-	g = alloc_disk_node(1 << PARTN_BITS,
-			pcibus_to_node(drive->hwif->pci_dev->bus));
+	if (drive->hwif->pci_dev) {
+		g = alloc_disk_node(1 << PARTN_BITS,
+				pcibus_to_node(drive->hwif->pci_dev->bus));
+	} else {
+		g = alloc_disk(1 << PARTN_BITS);
+	}
 	if (!g)
 		goto out_free_idkp;
 

Re: [PATCH] ide-disk oopses on boot

[Christoph Lameter]

On Tue, 9 Aug 2005, Petr Vandrovec wrote:

>   Problem is that pci_dev may be NULL - and it is NULL for example with
> kernel I've just built, with amd IDE driver built as a module while with
> ide-disk built into the kernel.

Yes that was discussed extensively by Andi and me and finally fixed by 
Kiran's patch in 2.6.13-rc6.

Re: [PATCH] ide-disk oopses on boot

[Christoph Lameter]

On Wed, 10 Aug 2005, Petr Vandrovec wrote:

> > Yes that was discussed extensively by Andi and me and finally fixed by
> > Kiran's patch in 2.6.13-rc6.
> 
> By which patch?  I hit it with post-2.6.13-rc6, exactly 2.6.13-rc6 with
> checkin hash "commit 00dd1e433967872f3997a45d5adf35056fdf2f56".  So if it is
> supposed to be fixed in 2.6.13-rc6, it is not.

Yes you are right there is one additional place where pcibus_to_node is 
used with the hwif that we did not cover. This better go into 2.6.13.

---
Fix ide-disk.c oops caused by hwif == NULL

1. Move hwif_to_node to ide.h

2. Use hwif_to_node in ide-disk.c

Signed-off-by: Christoph Lameter <clameter@sgi.com>

Index: linux-2.6/drivers/ide/ide-disk.c
===================================================================
--- linux-2.6.orig/drivers/ide/ide-disk.c	2005-07-27 18:29:17.000000000 -0700
+++ linux-2.6/drivers/ide/ide-disk.c	2005-08-09 19:55:03.000000000 -0700
@@ -1220,7 +1220,7 @@
 		goto failed;
 
 	g = alloc_disk_node(1 << PARTN_BITS,
-			pcibus_to_node(drive->hwif->pci_dev->bus));
+			hwif_to_node(drive->hwif));
 	if (!g)
 		goto out_free_idkp;
 
Index: linux-2.6/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.orig/drivers/ide/ide-probe.c	2005-08-04 15:47:15.000000000 -0700
+++ linux-2.6/drivers/ide/ide-probe.c	2005-08-09 19:46:50.000000000 -0700
@@ -960,15 +960,6 @@
 }
 #endif /* MAX_HWIFS > 1 */
 
-static inline int hwif_to_node(ide_hwif_t *hwif)
-{
-	if (hwif->pci_dev)
-		return pcibus_to_node(hwif->pci_dev->bus);
-	else
-		/* Add ways to determine the node of other busses here */
-		return -1;
-}
-
 /*
  * init request queue
  */
Index: linux-2.6/include/linux/ide.h
===================================================================
--- linux-2.6.orig/include/linux/ide.h	2005-07-27 18:29:23.000000000 -0700
+++ linux-2.6/include/linux/ide.h	2005-08-09 19:47:14.000000000 -0700
@@ -1501,4 +1501,13 @@
 #define ide_id_has_flush_cache_ext(id)	\
 	(((id)->cfs_enable_2 & 0x2400) == 0x2400)
 
+static inline int hwif_to_node(ide_hwif_t *hwif)
+{
+	if (hwif->pci_dev)
+		return pcibus_to_node(hwif->pci_dev->bus);
+	else
+		/* Add ways to determine the node of other busses here */
+		return -1;
+}
+
 #endif /* _IDE_H */

Re: [PATCH] ide-disk oopses on boot

[Bartlomiej Zolnierkiewicz]

Why have you removed me from cc:?

On 8/10/05, Christoph Lameter <christoph@lameter.com> wrote:
> On Wed, 10 Aug 2005, Petr Vandrovec wrote:
> 
> > > Yes that was discussed extensively by Andi and me and finally fixed by
> > > Kiran's patch in 2.6.13-rc6.
> >
> > By which patch?  I hit it with post-2.6.13-rc6, exactly 2.6.13-rc6 with
> > checkin hash "commit 00dd1e433967872f3997a45d5adf35056fdf2f56".  So if it is
> > supposed to be fixed in 2.6.13-rc6, it is not.
> 
> Yes you are right there is one additional place where pcibus_to_node is
> used with the hwif that we did not cover. This better go into 2.6.13.

Yes.

> ---
> Fix ide-disk.c oops caused by hwif == NULL

This description is wrong, should be: hwif->pci_dev == NULL

> 1. Move hwif_to_node to ide.h
> 
> 2. Use hwif_to_node in ide-disk.c
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> Index: linux-2.6/drivers/ide/ide-disk.c
> ===================================================================
> --- linux-2.6.orig/drivers/ide/ide-disk.c       2005-07-27 18:29:17.000000000 -0700
> +++ linux-2.6/drivers/ide/ide-disk.c    2005-08-09 19:55:03.000000000 -0700
> @@ -1220,7 +1220,7 @@
>                 goto failed;
> 
>         g = alloc_disk_node(1 << PARTN_BITS,
> -                       pcibus_to_node(drive->hwif->pci_dev->bus));
> +                       hwif_to_node(drive->hwif));
>         if (!g)
>                 goto out_free_idkp;
> 
> Index: linux-2.6/drivers/ide/ide-probe.c
> ===================================================================
> --- linux-2.6.orig/drivers/ide/ide-probe.c      2005-08-04 15:47:15.000000000 -0700
> +++ linux-2.6/drivers/ide/ide-probe.c   2005-08-09 19:46:50.000000000 -0700
> @@ -960,15 +960,6 @@
>  }
>  #endif /* MAX_HWIFS > 1 */
> 
> -static inline int hwif_to_node(ide_hwif_t *hwif)
> -{
> -       if (hwif->pci_dev)
> -               return pcibus_to_node(hwif->pci_dev->bus);
> -       else
> -               /* Add ways to determine the node of other busses here */
> -               return -1;
> -}
> -
>  /*
>   * init request queue
>   */
> Index: linux-2.6/include/linux/ide.h
> ===================================================================
> --- linux-2.6.orig/include/linux/ide.h  2005-07-27 18:29:23.000000000 -0700
> +++ linux-2.6/include/linux/ide.h       2005-08-09 19:47:14.000000000 -0700
> @@ -1501,4 +1501,13 @@
>  #define ide_id_has_flush_cache_ext(id) \
>         (((id)->cfs_enable_2 & 0x2400) == 0x2400)
> 
> +static inline int hwif_to_node(ide_hwif_t *hwif)
> +{
> +       if (hwif->pci_dev)
> +               return pcibus_to_node(hwif->pci_dev->bus);
> +       else
> +               /* Add ways to determine the node of other busses here */
> +               return -1;
> +}
> +
>  #endif /* _IDE_H */
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] ide-disk oopses on boot

[Alan Cox]

On Maw, 2005-08-09 at 19:59 -0700, Christoph Lameter wrote:
> Yes you are right there is one additional place where pcibus_to_node is 
> used with the hwif that we did not cover. This better go into 2.6.13.

drive->hwif is not permitted to be NULL. Please work back and fix the
actual bug.

> 

Re: [PATCH] ide-disk oopses on boot

[Christoph Lameter]

On Wed, 10 Aug 2005, Alan Cox wrote:

> On Maw, 2005-08-09 at 19:59 -0700, Christoph Lameter wrote:
> > Yes you are right there is one additional place where pcibus_to_node is 
> > used with the hwif that we did not cover. This better go into 2.6.13.
> 
> drive->hwif is not permitted to be NULL. Please work back and fix the
> actual bug.

The actual bug was fixed. The description was wrong. We are not checking 
hwif but hwif->pci_dev.