2005-12-07 22:34:24

by Ravikiran G Thirumalai

[permalink] [raw]
Subject: pcibus_to_node value when no pxm info is present for the pci bus

Most of the arches seem to return -1 for pcibus_to_node if there is no pxm
kind of proximity information for the pci busses. Arch specific code on
those arches check if nodeid >= 0 before using the nodeid for kmalloc_node
etc. But some code paths in x86_64/i386 does not doe this --
x86_64/dma_alloc_pages() and e1000 node local descriptor (I am to blame for
the second one). Also, pcibus_to_node seems to be 0 when there is no pxm
info available.

The question is, what should be the default pcibus_to_node if there is no
pxm info? Answer seems like -1 -- in which case dma_alloc_pages and e1000
driver has to be fixed.

Thanks,
Kiran


2005-12-08 19:05:32

by Christoph Lameter

[permalink] [raw]
Subject: Re: pcibus_to_node value when no pxm info is present for the pci bus

On Wed, 7 Dec 2005, Ravikiran G Thirumalai wrote:

> Most of the arches seem to return -1 for pcibus_to_node if there is no pxm
> kind of proximity information for the pci busses. Arch specific code on
> those arches check if nodeid >= 0 before using the nodeid for kmalloc_node
> etc. But some code paths in x86_64/i386 does not doe this --
> x86_64/dma_alloc_pages() and e1000 node local descriptor (I am to blame for
> the second one). Also, pcibus_to_node seems to be 0 when there is no pxm
> info available.

kmalloc_node falls back to kmalloc for node == -1. So there does not need
to be a check.

> The question is, what should be the default pcibus_to_node if there is no
> pxm info? Answer seems like -1 -- in which case dma_alloc_pages and e1000
> driver has to be fixed.

Why would they have to be fixed?

2005-12-08 19:34:49

by Ravikiran G Thirumalai

[permalink] [raw]
Subject: Re: pcibus_to_node value when no pxm info is present for the pci bus

On Thu, Dec 08, 2005 at 11:05:24AM -0800, Christoph Lameter wrote:
> On Wed, 7 Dec 2005, Ravikiran G Thirumalai wrote:
>
> > Most of the arches seem to return -1 for pcibus_to_node if there is no pxm
> > kind of proximity information for the pci busses. Arch specific code on
> > those arches check if nodeid >= 0 before using the nodeid for kmalloc_node
> > etc. But some code paths in x86_64/i386 does not doe this --
> > x86_64/dma_alloc_pages() and e1000 node local descriptor (I am to blame for
> > the second one). Also, pcibus_to_node seems to be 0 when there is no pxm
> > info available.
>
> kmalloc_node falls back to kmalloc for node == -1. So there does not need
> to be a check.

Ah! yes,

>
> > The question is, what should be the default pcibus_to_node if there is no
> > pxm info? Answer seems like -1 -- in which case dma_alloc_pages and e1000
> > driver has to be fixed.
>
> Why would they have to be fixed?

alloc_pages_node (used by dma_alloc_pages) does not seem to do the check
though. I guess alloc_pages_node needs to be fixed then.

Thanks,
Kiran

2005-12-08 20:04:43

by Andi Kleen

[permalink] [raw]
Subject: Re: [discuss] Re: pcibus_to_node value when no pxm info is present for the pci bus

> > > The question is, what should be the default pcibus_to_node if there is no
> > > pxm info? Answer seems like -1 -- in which case dma_alloc_pages and e1000
> > > driver has to be fixed.
> >
> > Why would they have to be fixed?
>
> alloc_pages_node (used by dma_alloc_pages) does not seem to do the check
> though. I guess alloc_pages_node needs to be fixed then.

Or just fix the caller. I will do that and change the default to
return -1 instead of 0.

Thanks,
-Andi

2005-12-08 20:21:49

by Ravikiran G Thirumalai

[permalink] [raw]
Subject: Re: [discuss] Re: pcibus_to_node value when no pxm info is present for the pci bus

On Thu, Dec 08, 2005 at 09:04:40PM +0100, Andi Kleen wrote:
> > > > The question is, what should be the default pcibus_to_node if there is no
> > > > pxm info? Answer seems like -1 -- in which case dma_alloc_pages and e1000
> > > > driver has to be fixed.
> > >
> > > Why would they have to be fixed?
> >
> > alloc_pages_node (used by dma_alloc_pages) does not seem to do the check
> > though. I guess alloc_pages_node needs to be fixed then.
>
> Or just fix the caller. I will do that and change the default to

That was my thinking earlier too, but shouldn't we have uniformity in
behaviour between kmalloc_node and alloc_pages_node wrt nodeid handling?
IMHO it would be less confusing that way. alloc_pages_node is not that much
of a fastpath routine anyways...

Thanks,
Kiran

2005-12-08 20:26:11

by Andi Kleen

[permalink] [raw]
Subject: Re: [discuss] Re: pcibus_to_node value when no pxm info is present for the pci bus

> That was my thinking earlier too, but shouldn't we have uniformity in
> behaviour between kmalloc_node and alloc_pages_node wrt nodeid handling?
> IMHO it would be less confusing that way. alloc_pages_node is not that much
> of a fastpath routine anyways...

No strong opinion either way.

-Andi

2005-12-08 20:41:34

by Christoph Lameter

[permalink] [raw]
Subject: Re: [discuss] Re: pcibus_to_node value when no pxm info is present for the pci bus

On Thu, 8 Dec 2005, Ravikiran G Thirumalai wrote:

> That was my thinking earlier too, but shouldn't we have uniformity in
> behaviour between kmalloc_node and alloc_pages_node wrt nodeid handling?
> IMHO it would be less confusing that way. alloc_pages_node is not that much
> of a fastpath routine anyways...

I agree.

2005-12-08 20:47:31

by Andi Kleen

[permalink] [raw]
Subject: Re: [discuss] Re: pcibus_to_node value when no pxm info is present for the pci bus

On Thu, Dec 08, 2005 at 12:41:28PM -0800, Christoph Lameter wrote:
> On Thu, 8 Dec 2005, Ravikiran G Thirumalai wrote:
>
> > That was my thinking earlier too, but shouldn't we have uniformity in
> > behaviour between kmalloc_node and alloc_pages_node wrt nodeid handling?
> > IMHO it would be less confusing that way. alloc_pages_node is not that much
> > of a fastpath routine anyways...
>
> I agree.

Ok I will submit a patch.

-Andi