* [PATCH] Fix crash on boot in kmalloc_node IDE changes
@ 2005-07-06 13:30 Andi Kleen
2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Andi Kleen @ 2005-07-06 13:30 UTC (permalink / raw)
To: linux-ide, torvalds, linux-kernel, christoph
Without this patch a dual Xeon EM64T machine would oops on boot
because the hwif pointer here was NULL. I also added a check for
pci_dev because it's doubtful that all IDE devices have pci_devs.
Signed-off-by: Andi Kleen <ak@suse.de>
Index: linux/drivers/ide/ide-probe.c
===================================================================
--- linux.orig/drivers/ide/ide-probe.c
+++ linux/drivers/ide/ide-probe.c
@@ -978,8 +978,10 @@ static int ide_init_queue(ide_drive_t *d
* do not.
*/
- q = blk_init_queue_node(do_ide_request, &ide_lock,
- pcibus_to_node(drive->hwif->pci_dev->bus));
+ int node = 0; /* Should be -1 */
+ if (drive->hwif && drive->hwif->pci_dev)
+ node = pcibus_to_node(drive->hwif->pci_dev->bus);
+ q = blk_init_queue_node(do_ide_request, &ide_lock, node);
if (!q)
return 1;
@@ -1096,8 +1098,13 @@ static int init_irq (ide_hwif_t *hwif)
hwgroup->hwif->next = hwif;
spin_unlock_irq(&ide_lock);
} else {
- hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,
- pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus));
+ int node = 0;
+ if (hwif->drives[0].hwif) {
+ struct pci_dev *pdev = hwif->drives[0].hwif->pci_dev;
+ if (pdev)
+ node = pcibus_to_node(pdev->bus);
+ }
+ hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,node);
if (!hwgroup)
goto out_up;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 13:30 [PATCH] Fix crash on boot in kmalloc_node IDE changes Andi Kleen
@ 2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
2005-07-06 14:09 ` Andi Kleen
2005-07-06 16:34 ` Christoph Lameter
2005-07-07 16:21 ` [another PATCH] " Christoph Lameter
2 siblings, 1 reply; 24+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-07-06 14:05 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel, christoph
On 7/6/05, Andi Kleen <ak@suse.de> wrote:
>
> Without this patch a dual Xeon EM64T machine would oops on boot
> because the hwif pointer here was NULL. I also added a check for
> pci_dev because it's doubtful that all IDE devices have pci_devs.
>
> Signed-off-by: Andi Kleen <ak@suse.de>
>
>
> Index: linux/drivers/ide/ide-probe.c
> ===================================================================
> --- linux.orig/drivers/ide/ide-probe.c
> +++ linux/drivers/ide/ide-probe.c
> @@ -978,8 +978,10 @@ static int ide_init_queue(ide_drive_t *d
> * do not.
> */
>
> - q = blk_init_queue_node(do_ide_request, &ide_lock,
> - pcibus_to_node(drive->hwif->pci_dev->bus));
> + int node = 0; /* Should be -1 */
> + if (drive->hwif && drive->hwif->pci_dev)
> + node = pcibus_to_node(drive->hwif->pci_dev->bus);
> + q = blk_init_queue_node(do_ide_request, &ide_lock, node);
> if (!q)
> return 1;
drive->hwif check is redundant, please remove it
> @@ -1096,8 +1098,13 @@ static int init_irq (ide_hwif_t *hwif)
> hwgroup->hwif->next = hwif;
> spin_unlock_irq(&ide_lock);
> } else {
> - hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,
> - pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus));
> + int node = 0;
> + if (hwif->drives[0].hwif) {
> + struct pci_dev *pdev = hwif->drives[0].hwif->pci_dev;
> + if (pdev)
> + node = pcibus_to_node(pdev->bus);
> + }
ditto, moreover hwif->drives[0].hwif == hwif
> + hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,node);
> if (!hwgroup)
> goto out_up;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
@ 2005-07-06 14:09 ` Andi Kleen
2005-07-06 14:35 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-06 14:09 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Andi Kleen, linux-ide, torvalds, linux-kernel, christoph
> drive->hwif check is redundant, please remove it
It's not. My first version didn't have it but it still crashed.
It's what actually prevents the crash.
I also don't know why, but it's true.
The machine had four IDE controllers BTW (on board an an external Promise
card)
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 14:09 ` Andi Kleen
@ 2005-07-06 14:35 ` Bartlomiej Zolnierkiewicz
2005-07-06 18:21 ` Andi Kleen
0 siblings, 1 reply; 24+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-07-06 14:35 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel, christoph
On 7/6/05, Andi Kleen <ak@suse.de> wrote:
> > drive->hwif check is redundant, please remove it
>
> It's not. My first version didn't have it but it still crashed.
> It's what actually prevents the crash.
> I also don't know why, but it's true.
very weird as HWIF(drive) == drive->hwif:
ide_hwif_t *hwif = HWIF(drive);
...
q = blk_init_queue_node(do_ide_request, &ide_lock,
pcibus_to_node(drive->hwif->pci_dev->bus));
if (!q)
return 1;
...
if (!hwif->rqsize) {
you should OOPS here
Bartlomiej
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 13:30 [PATCH] Fix crash on boot in kmalloc_node IDE changes Andi Kleen
2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
@ 2005-07-06 16:34 ` Christoph Lameter
2005-07-06 16:45 ` Andi Kleen
2005-07-07 16:21 ` [another PATCH] " Christoph Lameter
2 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2005-07-06 16:34 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel
On Wed, 6 Jul 2005, Andi Kleen wrote:
> - q = blk_init_queue_node(do_ide_request, &ide_lock,
> - pcibus_to_node(drive->hwif->pci_dev->bus));
> + int node = 0; /* Should be -1 */
Why is this not -1?
> + int node = 0;
> + if (hwif->drives[0].hwif) {
Also needs to be -1.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 16:34 ` Christoph Lameter
@ 2005-07-06 16:45 ` Andi Kleen
2005-07-06 17:07 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-06 16:45 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andi Kleen, linux-ide, torvalds, linux-kernel
On Wed, Jul 06, 2005 at 09:34:28AM -0700, Christoph Lameter wrote:
> On Wed, 6 Jul 2005, Andi Kleen wrote:
>
> > - q = blk_init_queue_node(do_ide_request, &ide_lock,
> > - pcibus_to_node(drive->hwif->pci_dev->bus));
> > + int node = 0; /* Should be -1 */
>
> Why is this not -1?
Because there is no code in rc3 that handles -1 in kmalloc_node.
If you add a patch that handles it then feel free to change.
But fixing the bootup has the highest priority.
>
> > + int node = 0;
> > + if (hwif->drives[0].hwif) {
>
> Also needs to be -1.
Then it would crash again.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 16:45 ` Andi Kleen
@ 2005-07-06 17:07 ` Christoph Lameter
0 siblings, 0 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-06 17:07 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel
On Wed, 6 Jul 2005, Andi Kleen wrote:
> On Wed, Jul 06, 2005 at 09:34:28AM -0700, Christoph Lameter wrote:
> > On Wed, 6 Jul 2005, Andi Kleen wrote:
> >
> > > - q = blk_init_queue_node(do_ide_request, &ide_lock,
> > > - pcibus_to_node(drive->hwif->pci_dev->bus));
> > > + int node = 0; /* Should be -1 */
> >
> > Why is this not -1?
>
> Because there is no code in rc3 that handles -1 in kmalloc_node.
>
> If you add a patch that handles it then feel free to change.
> But fixing the bootup has the highest priority.
>
> >
> > > + int node = 0;
> > > + if (hwif->drives[0].hwif) {
> >
> > Also needs to be -1.
>
> Then it would crash again.
The patch follows. Maybe add that and include a signoff by me?:
Index: linux-2.6.git/mm/slab.c
===================================================================
--- linux-2.6.git.orig/mm/slab.c 2005-07-05 17:03:30.000000000 -0700
+++ linux-2.6.git/mm/slab.c 2005-07-05 18:25:20.000000000 -0700
@@ -2372,6 +2372,9 @@
struct slab *slabp;
kmem_bufctl_t next;
+ if (nodeid == -1)
+ return kmem_cache_alloc_node(cachep, flags);
+
for (loop = 0;;loop++) {
struct list_head *q;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 14:35 ` Bartlomiej Zolnierkiewicz
@ 2005-07-06 18:21 ` Andi Kleen
0 siblings, 0 replies; 24+ messages in thread
From: Andi Kleen @ 2005-07-06 18:21 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, torvalds, linux-kernel, christoph
On Wed, 6 Jul 2005 16:35:11 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> On 7/6/05, Andi Kleen <ak@suse.de> wrote:
> > > drive->hwif check is redundant, please remove it
> >
> > It's not. My first version didn't have it but it still crashed.
> > It's what actually prevents the crash.
> > I also don't know why, but it's true.
>
> very weird as HWIF(drive) == drive->hwif:
>
> ide_hwif_t *hwif = HWIF(drive);
>
> ...
>
> q = blk_init_queue_node(do_ide_request, &ide_lock,
> pcibus_to_node(drive->hwif->pci_dev->bus));
^^^^^^^^^^^^^^
I oops on that one. Maybe it takes the early return.
> if (!q)
> return 1;
>
> ...
>
> if (!hwif->rqsize) {
>
> you should OOPS here
I would appreciate if some form of this patch is merged asap because it breaks
booting on my test machines.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-06 13:30 [PATCH] Fix crash on boot in kmalloc_node IDE changes Andi Kleen
2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
2005-07-06 16:34 ` Christoph Lameter
@ 2005-07-07 16:21 ` Christoph Lameter
2005-07-07 16:24 ` Andi Kleen
2005-07-07 16:38 ` Linus Torvalds
2 siblings, 2 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-07 16:21 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel
On Wed, 6 Jul 2005, Andi Kleen wrote:
> Without this patch a dual Xeon EM64T machine would oops on boot
> because the hwif pointer here was NULL. I also added a check for
> pci_dev because it's doubtful that all IDE devices have pci_devs.
Here is IMHO the right way to fix this. Test for the hwif != NULL and
test for pci_dev != NULL before determining the node number of the pci
bus that the device is connected to. Maybe we need a hwif_to_node for ide
drivers that is also able to determine the locality of other hardware?
Signed-off-by: Christoph Lameter <christoph@lameter.com>
Index: linux-2.6.git/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.git.orig/drivers/ide/ide-probe.c 2005-06-23 11:38:02.000000000 -0700
+++ linux-2.6.git/drivers/ide/ide-probe.c 2005-07-07 09:15:04.000000000 -0700
@@ -979,7 +979,8 @@
*/
q = blk_init_queue_node(do_ide_request, &ide_lock,
- pcibus_to_node(drive->hwif->pci_dev->bus));
+ (drive->hwif && drive->hwif->pci_dev) ?
+ pcibus_to_node(drive->hwif->pci_dev->bus): -1);
if (!q)
return 1;
@@ -1097,7 +1098,8 @@
spin_unlock_irq(&ide_lock);
} else {
hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,
- pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus));
+ (hwif->drives[0].hwif && hwif->drives[0].hwif->pci_dev) ?
+ pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus) : -1);
if (!hwgroup)
goto out_up;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:21 ` [another PATCH] " Christoph Lameter
@ 2005-07-07 16:24 ` Andi Kleen
2005-07-07 16:32 ` Christoph Lameter
2005-07-07 16:38 ` Linus Torvalds
1 sibling, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-07 16:24 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andi Kleen, linux-ide, torvalds, linux-kernel
On Thu, Jul 07, 2005 at 09:21:55AM -0700, Christoph Lameter wrote:
> On Wed, 6 Jul 2005, Andi Kleen wrote:
>
> > Without this patch a dual Xeon EM64T machine would oops on boot
> > because the hwif pointer here was NULL. I also added a check for
> > pci_dev because it's doubtful that all IDE devices have pci_devs.
>
> Here is IMHO the right way to fix this. Test for the hwif != NULL and
> test for pci_dev != NULL before determining the node number of the pci
> bus that the device is connected to. Maybe we need a hwif_to_node for ide
> drivers that is also able to determine the locality of other hardware?
Hmm? Where is the difference?
This is 100% equivalent to my code except that you compressed
it all into a single expression.
The former one was more readable.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:24 ` Andi Kleen
@ 2005-07-07 16:32 ` Christoph Lameter
2005-07-07 16:36 ` Andi Kleen
2005-07-07 16:46 ` Bartlomiej Zolnierkiewicz
0 siblings, 2 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-07 16:32 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel
On Thu, 7 Jul 2005, Andi Kleen wrote:
> On Thu, Jul 07, 2005 at 09:21:55AM -0700, Christoph Lameter wrote:
> > On Wed, 6 Jul 2005, Andi Kleen wrote:
> >
> > > Without this patch a dual Xeon EM64T machine would oops on boot
> > > because the hwif pointer here was NULL. I also added a check for
> > > pci_dev because it's doubtful that all IDE devices have pci_devs.
> >
> > Here is IMHO the right way to fix this. Test for the hwif != NULL and
> > test for pci_dev != NULL before determining the node number of the pci
> > bus that the device is connected to. Maybe we need a hwif_to_node for ide
> > drivers that is also able to determine the locality of other hardware?
>
> Hmm? Where is the difference?
node = -1 if the node cannot be determined.
> This is 100% equivalent to my code except that you compressed
> it all into a single expression.
My patch consistently checks for hwif != NULL and pci_dev != NULL.
There was someother stuff in your patch. This patch does not add any
additional variables and is more readable.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:32 ` Christoph Lameter
@ 2005-07-07 16:36 ` Andi Kleen
2005-07-07 17:15 ` Christoph Lameter
2005-07-07 16:46 ` Bartlomiej Zolnierkiewicz
1 sibling, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-07 16:36 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andi Kleen, linux-ide, torvalds, linux-kernel
On Thu, Jul 07, 2005 at 09:32:51AM -0700, Christoph Lameter wrote:
> On Thu, 7 Jul 2005, Andi Kleen wrote:
>
> > On Thu, Jul 07, 2005 at 09:21:55AM -0700, Christoph Lameter wrote:
> > > On Wed, 6 Jul 2005, Andi Kleen wrote:
> > >
> > > > Without this patch a dual Xeon EM64T machine would oops on boot
> > > > because the hwif pointer here was NULL. I also added a check for
> > > > pci_dev because it's doubtful that all IDE devices have pci_devs.
> > >
> > > Here is IMHO the right way to fix this. Test for the hwif != NULL and
> > > test for pci_dev != NULL before determining the node number of the pci
> > > bus that the device is connected to. Maybe we need a hwif_to_node for ide
> > > drivers that is also able to determine the locality of other hardware?
> >
> > Hmm? Where is the difference?
>
> node = -1 if the node cannot be determined.
But that will crash right now.
Trading one crash for another doesn't seem to be particularly useful.
> > This is 100% equivalent to my code except that you compressed
> > it all into a single expression.
>
> My patch consistently checks for hwif != NULL and pci_dev != NULL.
> There was someother stuff in your patch. This patch does not add any
> additional variables and is more readable.
No it was exactly the same except for a working node number.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:21 ` [another PATCH] " Christoph Lameter
2005-07-07 16:24 ` Andi Kleen
@ 2005-07-07 16:38 ` Linus Torvalds
2005-07-07 16:40 ` Andi Kleen
2005-07-07 17:23 ` Christoph Lameter
1 sibling, 2 replies; 24+ messages in thread
From: Linus Torvalds @ 2005-07-07 16:38 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andi Kleen, linux-ide, linux-kernel
On Thu, 7 Jul 2005, Christoph Lameter wrote:
>
> Here is IMHO the right way to fix this. Test for the hwif != NULL and
> test for pci_dev != NULL before determining the node number of the pci
> bus that the device is connected to.
I think this is pretty unreadable.
If you make it use a trivial inline function for the thing, I think that
would be ok, though.
Complex expressions are bad.
Linus
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:38 ` Linus Torvalds
@ 2005-07-07 16:40 ` Andi Kleen
2005-07-07 17:23 ` Christoph Lameter
1 sibling, 0 replies; 24+ messages in thread
From: Andi Kleen @ 2005-07-07 16:40 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Christoph Lameter, Andi Kleen, linux-ide, linux-kernel
On Thu, Jul 07, 2005 at 09:38:17AM -0700, Linus Torvalds wrote:
>
>
> On Thu, 7 Jul 2005, Christoph Lameter wrote:
> >
> > Here is IMHO the right way to fix this. Test for the hwif != NULL and
> > test for pci_dev != NULL before determining the node number of the pci
> > bus that the device is connected to.
>
> I think this is pretty unreadable.
>
> If you make it use a trivial inline function for the thing, I think that
> would be ok, though.
>
> Complex expressions are bad.
You could just use the previous patch I posted (hint hint...;)
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:32 ` Christoph Lameter
2005-07-07 16:36 ` Andi Kleen
@ 2005-07-07 16:46 ` Bartlomiej Zolnierkiewicz
1 sibling, 0 replies; 24+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-07-07 16:46 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Andi Kleen, linux-ide, torvalds, linux-kernel
On 7/7/05, Christoph Lameter <christoph@lameter.com> wrote:
> On Thu, 7 Jul 2005, Andi Kleen wrote:
>
> > On Thu, Jul 07, 2005 at 09:21:55AM -0700, Christoph Lameter wrote:
> > > On Wed, 6 Jul 2005, Andi Kleen wrote:
> > >
> > > > Without this patch a dual Xeon EM64T machine would oops on boot
> > > > because the hwif pointer here was NULL. I also added a check for
> > > > pci_dev because it's doubtful that all IDE devices have pci_devs.
> > >
> > > Here is IMHO the right way to fix this. Test for the hwif != NULL and
> > > test for pci_dev != NULL before determining the node number of the pci
> > > bus that the device is connected to. Maybe we need a hwif_to_node for ide
> > > drivers that is also able to determine the locality of other hardware?
> >
> > Hmm? Where is the difference?
>
> node = -1 if the node cannot be determined.
>
> > This is 100% equivalent to my code except that you compressed
> > it all into a single expression.
>
> My patch consistently checks for hwif != NULL and pci_dev != NULL.
> There was someother stuff in your patch. This patch does not add any
> additional variables and is more readable.
seconded but hwif != NULL still just hides some other issue
* hwifs and drives are not allocated dynamically
* drive->hwif is initialized to hwif for every hwif very early in the
driver initialization
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:36 ` Andi Kleen
@ 2005-07-07 17:15 ` Christoph Lameter
0 siblings, 0 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-07 17:15 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-ide, torvalds, linux-kernel
On Thu, 7 Jul 2005, Andi Kleen wrote:
> > node = -1 if the node cannot be determined.
>
> But that will crash right now.
That was fixed. Have a look at the git logs.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 16:38 ` Linus Torvalds
2005-07-07 16:40 ` Andi Kleen
@ 2005-07-07 17:23 ` Christoph Lameter
2005-07-07 17:31 ` Jens Axboe
2005-07-07 18:57 ` Linus Torvalds
1 sibling, 2 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-07 17:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Andi Kleen, linux-ide, linux-kernel
On Thu, 7 Jul 2005, Linus Torvalds wrote:
> If you make it use a trivial inline function for the thing, I think that
> would be ok, though.
Like this?
Index: linux-2.6.git/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.git.orig/drivers/ide/ide-probe.c 2005-06-23 11:38:02.000000000 -0700
+++ linux-2.6.git/drivers/ide/ide-probe.c 2005-07-07 10:22:02.000000000 -0700
@@ -960,6 +960,15 @@
}
#endif /* MAX_HWIFS > 1 */
+static inline int hwif_to_node(ide_hwif_t *hwif)
+{
+ if (hwif && 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
*/
@@ -978,8 +987,7 @@
* do not.
*/
- q = blk_init_queue_node(do_ide_request, &ide_lock,
- pcibus_to_node(drive->hwif->pci_dev->bus));
+ q = blk_init_queue_node(do_ide_request, &ide_lock, hwif_to_node(hwif));
if (!q)
return 1;
@@ -1097,7 +1105,7 @@
spin_unlock_irq(&ide_lock);
} else {
hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,
- pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus));
+ hwif_to_node(hwif->drives[0].hwif));
if (!hwgroup)
goto out_up;
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 17:23 ` Christoph Lameter
@ 2005-07-07 17:31 ` Jens Axboe
2005-07-07 18:57 ` Linus Torvalds
1 sibling, 0 replies; 24+ messages in thread
From: Jens Axboe @ 2005-07-07 17:31 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Linus Torvalds, Andi Kleen, linux-ide, linux-kernel
On Thu, Jul 07 2005, Christoph Lameter wrote:
> On Thu, 7 Jul 2005, Linus Torvalds wrote:
>
> > If you make it use a trivial inline function for the thing, I think that
> > would be ok, though.
>
> Like this?
>
> Index: linux-2.6.git/drivers/ide/ide-probe.c
> ===================================================================
> --- linux-2.6.git.orig/drivers/ide/ide-probe.c 2005-06-23 11:38:02.000000000 -0700
> +++ linux-2.6.git/drivers/ide/ide-probe.c 2005-07-07 10:22:02.000000000 -0700
> @@ -960,6 +960,15 @@
> }
> #endif /* MAX_HWIFS > 1 */
>
> +static inline int hwif_to_node(ide_hwif_t *hwif)
> +{
> + if (hwif && hwif->pci_dev)
> + return pcibus_to_node(hwif->pci_dev->bus);
> + else
> + /* Add ways to determine the node of other busses here */
> + return -1;
> +}
When is hwif ever NULL?
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 17:23 ` Christoph Lameter
2005-07-07 17:31 ` Jens Axboe
@ 2005-07-07 18:57 ` Linus Torvalds
2005-07-07 19:09 ` Christoph Lameter
1 sibling, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2005-07-07 18:57 UTC (permalink / raw)
To: Christoph Lameter, Bartlomiej Zolnierkiewicz
Cc: Andi Kleen, linux-ide, Linux Kernel Mailing List
On Thu, 7 Jul 2005, Christoph Lameter wrote:
>
> On Thu, 7 Jul 2005, Linus Torvalds wrote:
>
> > If you make it use a trivial inline function for the thing, I think that
> > would be ok, though.
>
> Like this?
Yes. Except that if hwif is NULL, we'll have other oopses since we access
that in other places.
Why _is_ hwif NULL anyway? That's another, unrelated thing, and should
probably have a separate check and an early return.
So there's two unrelated issues: hwif being NULL is a serious error
regardless of any node issues, and pci_dev being NULL just means we aren't
a PCI device.
Bartlomiej?
Linus
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 18:57 ` Linus Torvalds
@ 2005-07-07 19:09 ` Christoph Lameter
2005-07-07 21:15 ` Andi Kleen
0 siblings, 1 reply; 24+ messages in thread
From: Christoph Lameter @ 2005-07-07 19:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Bartlomiej Zolnierkiewicz, Andi Kleen, linux-ide,
Linux Kernel Mailing List
On Thu, 7 Jul 2005, Linus Torvalds wrote:
> Yes. Except that if hwif is NULL, we'll have other oopses since we access
> that in other places.
>
> Why _is_ hwif NULL anyway? That's another, unrelated thing, and should
> probably have a separate check and an early return.
I was wondering about that one as well. Andi brought it up.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 19:09 ` Christoph Lameter
@ 2005-07-07 21:15 ` Andi Kleen
2005-07-07 21:19 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-07 21:15 UTC (permalink / raw)
To: Christoph Lameter
Cc: Linus Torvalds, Bartlomiej Zolnierkiewicz, Andi Kleen, linux-ide,
Linux Kernel Mailing List
On Thu, Jul 07, 2005 at 12:09:00PM -0700, Christoph Lameter wrote:
> On Thu, 7 Jul 2005, Linus Torvalds wrote:
>
> > Yes. Except that if hwif is NULL, we'll have other oopses since we access
> > that in other places.
> >
> > Why _is_ hwif NULL anyway? That's another, unrelated thing, and should
> > probably have a separate check and an early return.
>
> I was wondering about that one as well. Andi brought it up.
I don't know why hwif was NULL, but my kernel definitely crashed.
hwif was NULL in the first function (I first misread the oops
and thought it was pci_dev NULL, but it wasn't). For the second
I didn't verify it was hwif or pci_dev NULL, but one of them
was too.
The setup was a Intel board with 1 PATA/4 SATA onboard and only a CD-ROM
and a external Promise PATA controller with two PATA disks.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 21:15 ` Andi Kleen
@ 2005-07-07 21:19 ` Bartlomiej Zolnierkiewicz
2005-07-07 21:25 ` Andi Kleen
0 siblings, 1 reply; 24+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-07-07 21:19 UTC (permalink / raw)
To: Andi Kleen
Cc: Christoph Lameter, Linus Torvalds, linux-ide,
Linux Kernel Mailing List
On 7/7/05, Andi Kleen <ak@suse.de> wrote:
> On Thu, Jul 07, 2005 at 12:09:00PM -0700, Christoph Lameter wrote:
> > On Thu, 7 Jul 2005, Linus Torvalds wrote:
> >
> > > Yes. Except that if hwif is NULL, we'll have other oopses since we access
> > > that in other places.
> > >
> > > Why _is_ hwif NULL anyway? That's another, unrelated thing, and should
> > > probably have a separate check and an early return.
> >
> > I was wondering about that one as well. Andi brought it up.
>
> I don't know why hwif was NULL, but my kernel definitely crashed.
> hwif was NULL in the first function (I first misread the oops
> and thought it was pci_dev NULL, but it wasn't). For the second
> I didn't verify it was hwif or pci_dev NULL, but one of them
> was too.
>
> The setup was a Intel board with 1 PATA/4 SATA onboard and only a CD-ROM
> and a external Promise PATA controller with two PATA disks.
actual OOPS would be very useful
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 21:19 ` Bartlomiej Zolnierkiewicz
@ 2005-07-07 21:25 ` Andi Kleen
2005-07-08 4:44 ` Christoph Lameter
0 siblings, 1 reply; 24+ messages in thread
From: Andi Kleen @ 2005-07-07 21:25 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Andi Kleen, Christoph Lameter, Linus Torvalds, linux-ide,
Linux Kernel Mailing List
> > The setup was a Intel board with 1 PATA/4 SATA onboard and only a CD-ROM
> > and a external Promise PATA controller with two PATA disks.
>
> actual OOPS would be very useful
It's difficult because I don't have serial on that machine.
-Andi
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [another PATCH] Fix crash on boot in kmalloc_node IDE changes
2005-07-07 21:25 ` Andi Kleen
@ 2005-07-08 4:44 ` Christoph Lameter
0 siblings, 0 replies; 24+ messages in thread
From: Christoph Lameter @ 2005-07-08 4:44 UTC (permalink / raw)
To: Andi Kleen
Cc: Bartlomiej Zolnierkiewicz, Linus Torvalds, linux-ide,
Linux Kernel Mailing List
On Thu, 7 Jul 2005, Andi Kleen wrote:
> > > The setup was a Intel board with 1 PATA/4 SATA onboard and only a CD-ROM
> > > and a external Promise PATA controller with two PATA disks.
> >
> > actual OOPS would be very useful
>
> It's difficult because I don't have serial on that machine.
Maybe we can settle on this patch until we know for sure that the hwif
is also a problem. In that case we can simply patch hwif_to_node:
Index: linux-2.6.git/drivers/ide/ide-probe.c
===================================================================
--- linux-2.6.git.orig/drivers/ide/ide-probe.c 2005-06-23 11:38:02.000000000 -0700
+++ linux-2.6.git/drivers/ide/ide-probe.c 2005-07-07 20:18:22.000000000 -0700
@@ -960,6 +960,15 @@
}
#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
*/
@@ -978,8 +987,7 @@
* do not.
*/
- q = blk_init_queue_node(do_ide_request, &ide_lock,
- pcibus_to_node(drive->hwif->pci_dev->bus));
+ q = blk_init_queue_node(do_ide_request, &ide_lock, hwif_to_node(hwif));
if (!q)
return 1;
@@ -1097,7 +1105,7 @@
spin_unlock_irq(&ide_lock);
} else {
hwgroup = kmalloc_node(sizeof(ide_hwgroup_t), GFP_KERNEL,
- pcibus_to_node(hwif->drives[0].hwif->pci_dev->bus));
+ hwif_to_node(hwif->drives[0].hwif));
if (!hwgroup)
goto out_up;
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2005-07-08 4:44 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-06 13:30 [PATCH] Fix crash on boot in kmalloc_node IDE changes Andi Kleen
2005-07-06 14:05 ` Bartlomiej Zolnierkiewicz
2005-07-06 14:09 ` Andi Kleen
2005-07-06 14:35 ` Bartlomiej Zolnierkiewicz
2005-07-06 18:21 ` Andi Kleen
2005-07-06 16:34 ` Christoph Lameter
2005-07-06 16:45 ` Andi Kleen
2005-07-06 17:07 ` Christoph Lameter
2005-07-07 16:21 ` [another PATCH] " Christoph Lameter
2005-07-07 16:24 ` Andi Kleen
2005-07-07 16:32 ` Christoph Lameter
2005-07-07 16:36 ` Andi Kleen
2005-07-07 17:15 ` Christoph Lameter
2005-07-07 16:46 ` Bartlomiej Zolnierkiewicz
2005-07-07 16:38 ` Linus Torvalds
2005-07-07 16:40 ` Andi Kleen
2005-07-07 17:23 ` Christoph Lameter
2005-07-07 17:31 ` Jens Axboe
2005-07-07 18:57 ` Linus Torvalds
2005-07-07 19:09 ` Christoph Lameter
2005-07-07 21:15 ` Andi Kleen
2005-07-07 21:19 ` Bartlomiej Zolnierkiewicz
2005-07-07 21:25 ` Andi Kleen
2005-07-08 4:44 ` Christoph Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).