Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Monday 11 February 2008, Andi Kleen wrote:
>
> Without this patch a Opteron test system here oopses at boot with currentg
> git.
>
> Calling to_pci_dev() on a NULL pointer gives a negative value so the
> following NULL
> pointer check never triggers and then an illegal address is referenced. Check
> the
> unadjusted original device pointer for NULL instead.
>
> Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
Thanks Andi!
This may explain+resolve ide_generic OOPS-es reported by Pavel/Kamalesh/Nish.
[ I also see that Linus has already merged the patch and you've followed
up on pcibus_to_node() issue so there is nothing left for me to do...
I really love when things turn out like this... ;) ]
> Index: linux/include/linux/ide.h
> ===
> --- linux.orig/include/linux/ide.h
> +++ linux/include/linux/ide.h
> @@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
> static inline int hwif_to_node(ide_hwif_t *hwif)
> {
> struct pci_dev *dev = to_pci_dev(hwif->dev);
> - return dev ? pcibus_to_node(dev->bus) : -1;
> + return hwif->dev ? pcibus_to_node(dev->bus) : -1;
> }
>
> static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, Feb 11, 2008 at 09:37:18AM -0800, Linus Torvalds wrote: > > > On Mon, 11 Feb 2008, Linus Torvalds wrote: > > > > So we should probably make pcibus_to_node() be an inline function for that > > case > > Or, we could just do the ugliest patch ever, namely > > -#define pcibus_to_node(node) (-1) > +#define pcibus_to_node(node) ((int)(long)(node),-1) > > Wow. It's so ugly it's almost wraps around and comes out the other sides > and looks pretty. (void)arg, ... is better. Trick originally from Jan Beulich I think (his code is always a good source for useful new C tricks) -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, Feb 11, 2008 at 09:33:14AM -0800, Linus Torvalds wrote:
>
>
> On Mon, 11 Feb 2008, Andi Kleen wrote:
> >
> > Without this patch a Opteron test system here oopses at boot with currentg
> > git.
> >
> > Calling to_pci_dev() on a NULL pointer gives a negative value so the
> > following NULL
> > pointer check never triggers and then an illegal address is referenced.
> > Check the
> > unadjusted original device pointer for NULL instead.
> >
> > Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
> >
> > Index: linux/include/linux/ide.h
> > ===
> > --- linux.orig/include/linux/ide.h
> > +++ linux/include/linux/ide.h
> > @@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
> > static inline int hwif_to_node(ide_hwif_t *hwif)
> > {
> > struct pci_dev *dev = to_pci_dev(hwif->dev);
> > - return dev ? pcibus_to_node(dev->bus) : -1;
> > + return hwif->dev ? pcibus_to_node(dev->bus) : -1;
> > }
>
> Ok, I applied this, but it causes a *lot* of noise about "unused variable
> 'dev'" because pcibus_to_node() is defined to be -1 when you don't do any
> strange NUMA thing (so that "dev->bus" usage thing is never even seen by
> the compiler.
>
> So we should probably make pcibus_to_node() be an inline function for that
> case, or just make that thing be
>
> return hwif->dev ?
> pcibus_to_node(to_pci_dev(hwif->dev)->bus)
> :
> -1;
>
> or something. Because now things are really noisy.
Yes sorry; I only checked NUMA builds before sending it off.
The easiest way is probably just the traditional (void) cast
I fixed most of the topology macros while I was at it.
-Andi
---
Make topology fallback macros reference their arguments.
This avoids warnings with unreferenced variables in the !NUMA case.
Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
Index: linux/include/asm-generic/topology.h
===
--- linux.orig/include/asm-generic/topology.h
+++ linux/include/asm-generic/topology.h
@@ -30,19 +30,19 @@
/* Other architectures wishing to use this simple topology API should fill
in the below functions as appropriate in their own file. */
#ifndef cpu_to_node
-#define cpu_to_node(cpu) (0)
+#define cpu_to_node(cpu) ((void)(cpu),0)
#endif
#ifndef parent_node
-#define parent_node(node) (0)
+#define parent_node(node) ((void)(node),0)
#endif
#ifndef node_to_cpumask
-#define node_to_cpumask(node) (cpu_online_map)
+#define node_to_cpumask(node) ((void)node, cpu_online_map)
#endif
#ifndef node_to_first_cpu
-#define node_to_first_cpu(node)(0)
+#define node_to_first_cpu(node)((void)(node),0)
#endif
#ifndef pcibus_to_node
-#define pcibus_to_node(node) (-1)
+#define pcibus_to_node(bus)((void)(bus), -1)
#endif
#ifndef pcibus_to_cpumask
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, 11 Feb 2008, Linus Torvalds wrote: > > So we should probably make pcibus_to_node() be an inline function for that > case Or, we could just do the ugliest patch ever, namely -#define pcibus_to_node(node) (-1) +#define pcibus_to_node(node) ((int)(long)(node),-1) Wow. It's so ugly it's almost wraps around and comes out the other sides and looks pretty. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, 11 Feb 2008, Andi Kleen wrote:
>
> Without this patch a Opteron test system here oopses at boot with currentg
> git.
>
> Calling to_pci_dev() on a NULL pointer gives a negative value so the
> following NULL
> pointer check never triggers and then an illegal address is referenced. Check
> the
> unadjusted original device pointer for NULL instead.
>
> Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
>
> Index: linux/include/linux/ide.h
> ===
> --- linux.orig/include/linux/ide.h
> +++ linux/include/linux/ide.h
> @@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
> static inline int hwif_to_node(ide_hwif_t *hwif)
> {
> struct pci_dev *dev = to_pci_dev(hwif->dev);
> - return dev ? pcibus_to_node(dev->bus) : -1;
> + return hwif->dev ? pcibus_to_node(dev->bus) : -1;
> }
Ok, I applied this, but it causes a *lot* of noise about "unused variable
'dev'" because pcibus_to_node() is defined to be -1 when you don't do any
strange NUMA thing (so that "dev->bus" usage thing is never even seen by
the compiler.
So we should probably make pcibus_to_node() be an inline function for that
case, or just make that thing be
return hwif->dev ?
pcibus_to_node(to_pci_dev(hwif->dev)->bus)
:
-1;
or something. Because now things are really noisy.
Preferences, anybody? Bartlomiej?
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Monday 11 February 2008, Andi Kleen wrote:
Without this patch a Opteron test system here oopses at boot with currentg
git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the
following NULL
pointer check never triggers and then an illegal address is referenced. Check
the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Thanks Andi!
This may explain+resolve ide_generic OOPS-es reported by Pavel/Kamalesh/Nish.
[ I also see that Linus has already merged the patch and you've followed
up on pcibus_to_node() issue so there is nothing left for me to do...
I really love when things turn out like this... ;) ]
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif-dev);
- return dev ? pcibus_to_node(dev-bus) : -1;
+ return hwif-dev ? pcibus_to_node(dev-bus) : -1;
}
static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, 11 Feb 2008, Linus Torvalds wrote: So we should probably make pcibus_to_node() be an inline function for that case Or, we could just do the ugliest patch ever, namely -#define pcibus_to_node(node) (-1) +#define pcibus_to_node(node) ((int)(long)(node),-1) Wow. It's so ugly it's almost wraps around and comes out the other sides and looks pretty. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, 11 Feb 2008, Andi Kleen wrote:
Without this patch a Opteron test system here oopses at boot with currentg
git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the
following NULL
pointer check never triggers and then an illegal address is referenced. Check
the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif-dev);
- return dev ? pcibus_to_node(dev-bus) : -1;
+ return hwif-dev ? pcibus_to_node(dev-bus) : -1;
}
Ok, I applied this, but it causes a *lot* of noise about unused variable
'dev' because pcibus_to_node() is defined to be -1 when you don't do any
strange NUMA thing (so that dev-bus usage thing is never even seen by
the compiler.
So we should probably make pcibus_to_node() be an inline function for that
case, or just make that thing be
return hwif-dev ?
pcibus_to_node(to_pci_dev(hwif-dev)-bus)
:
-1;
or something. Because now things are really noisy.
Preferences, anybody? Bartlomiej?
Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, Feb 11, 2008 at 09:37:18AM -0800, Linus Torvalds wrote: On Mon, 11 Feb 2008, Linus Torvalds wrote: So we should probably make pcibus_to_node() be an inline function for that case Or, we could just do the ugliest patch ever, namely -#define pcibus_to_node(node) (-1) +#define pcibus_to_node(node) ((int)(long)(node),-1) Wow. It's so ugly it's almost wraps around and comes out the other sides and looks pretty. (void)arg, ... is better. Trick originally from Jan Beulich I think (his code is always a good source for useful new C tricks) -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Prevent IDE boot ops on NUMA system in mainline
On Mon, Feb 11, 2008 at 09:33:14AM -0800, Linus Torvalds wrote:
On Mon, 11 Feb 2008, Andi Kleen wrote:
Without this patch a Opteron test system here oopses at boot with currentg
git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the
following NULL
pointer check never triggers and then an illegal address is referenced.
Check the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif-dev);
- return dev ? pcibus_to_node(dev-bus) : -1;
+ return hwif-dev ? pcibus_to_node(dev-bus) : -1;
}
Ok, I applied this, but it causes a *lot* of noise about unused variable
'dev' because pcibus_to_node() is defined to be -1 when you don't do any
strange NUMA thing (so that dev-bus usage thing is never even seen by
the compiler.
So we should probably make pcibus_to_node() be an inline function for that
case, or just make that thing be
return hwif-dev ?
pcibus_to_node(to_pci_dev(hwif-dev)-bus)
:
-1;
or something. Because now things are really noisy.
Yes sorry; I only checked NUMA builds before sending it off.
The easiest way is probably just the traditional (void) cast
I fixed most of the topology macros while I was at it.
-Andi
---
Make topology fallback macros reference their arguments.
This avoids warnings with unreferenced variables in the !NUMA case.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Index: linux/include/asm-generic/topology.h
===
--- linux.orig/include/asm-generic/topology.h
+++ linux/include/asm-generic/topology.h
@@ -30,19 +30,19 @@
/* Other architectures wishing to use this simple topology API should fill
in the below functions as appropriate in their own asm/topology.h file. */
#ifndef cpu_to_node
-#define cpu_to_node(cpu) (0)
+#define cpu_to_node(cpu) ((void)(cpu),0)
#endif
#ifndef parent_node
-#define parent_node(node) (0)
+#define parent_node(node) ((void)(node),0)
#endif
#ifndef node_to_cpumask
-#define node_to_cpumask(node) (cpu_online_map)
+#define node_to_cpumask(node) ((void)node, cpu_online_map)
#endif
#ifndef node_to_first_cpu
-#define node_to_first_cpu(node)(0)
+#define node_to_first_cpu(node)((void)(node),0)
#endif
#ifndef pcibus_to_node
-#define pcibus_to_node(node) (-1)
+#define pcibus_to_node(bus)((void)(bus), -1)
#endif
#ifndef pcibus_to_cpumask
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Prevent IDE boot ops on NUMA system in mainline
Without this patch a Opteron test system here oopses at boot with currentg git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the following
NULL
pointer check never triggers and then an illegal address is referenced. Check
the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif->dev);
- return dev ? pcibus_to_node(dev->bus) : -1;
+ return hwif->dev ? pcibus_to_node(dev->bus) : -1;
}
static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Prevent IDE boot ops on NUMA system in mainline
Without this patch a Opteron test system here oopses at boot with currentg git.
Calling to_pci_dev() on a NULL pointer gives a negative value so the following
NULL
pointer check never triggers and then an illegal address is referenced. Check
the
unadjusted original device pointer for NULL instead.
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Index: linux/include/linux/ide.h
===
--- linux.orig/include/linux/ide.h
+++ linux/include/linux/ide.h
@@ -1294,7 +1294,7 @@ static inline void ide_dump_identify(u8
static inline int hwif_to_node(ide_hwif_t *hwif)
{
struct pci_dev *dev = to_pci_dev(hwif-dev);
- return dev ? pcibus_to_node(dev-bus) : -1;
+ return hwif-dev ? pcibus_to_node(dev-bus) : -1;
}
static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
