Re: 2.6.12-rc4-mm2 build failure

[Matthew Dobson <colpatch@us.ibm.com>]

Christoph Lameter wrote:
> On Wed, 18 May 2005, Matthew Dobson wrote:
> 
> 
>>I think you explained it well yourself.  If another function needs to be
>>added for ALL ARCHES, then ALL ARCHES will need to add the function.  In
>>most cases there is no single function that is both CORRECT and GENERIC
>>across all arches.  The way that i386, ia64, ppc64, etc. will map PCI Buses
>>to Nodes (for instance) will NOT be the same.  Anyone who adds a new
>>topology function has the responsibility of
>>1) making sure it works for all arches which support topology, or
>>2) getting the arch maintainers involved and helping them make sure it
>>works for all arches.
> 
> 
> The topology function in generic may just do nothing
> if not defined by the arch like pcibus_to_node. If the arch does not
> define it return indeterminate and make all node specific allocations fall
> back to unspecific allocation. This works for all arches and preserves
> the existing behavior.

Which is the way it was designed.  Arches which just want a safe/sane
default (yet likely inefficient) topology behavior that makes the system
look like a UP/SMP box should include asm-generic/topology.h from their own
asm/topology.h.  Arches that want to provide a useful and accurate topology
behavior should define their own implementation of the functions in
asm/topology.h.

The compilation breakage from defining a new topology function for some
arches and using it in generic code without defining it for all arches that
implement topology was intentional.  It's a sign that whoever made that new
function should at least solicit some feedback from other arches to make
sure that they are doing something sane, and so that we just don't add new
topology functions on a whim.  It was hoped that this would encourage some
discussion amongst arches to come up with interfaces that would be usable
across multiple (all?) arches.


>>New topology functions don't really get added all that often.  We've got
>>the basics (CPU, Mem, I/O Buses, Nodes) mapped in various ways, so there
>>shouldn't be tons of new functions added.  If someone wants to add a new
>>function, it's their responsibility to make sure that it doesn't break
>>anyone's arch.
> 
> 
> Its best to have the kernel setup in such a way that functions can be 
> added without having to cause breakage.
> 
> It is imaginable that someone will add more hardware something to node
> functions in the near future given that pcibus_to_node is just for pci.

I agree.  The way (historically) to not cause breakage is to implement your
new topology function for arches that care, or just stick a sane default in
their topology.h.  Every arch does it the way I'm suggesting, except IA64 &
x86_64.

x86_64 surprisingly does define pcibus_to_cpumask, but NOT
pcibus_to_node().  That means when they unconditionally #include
<asm-generic/topology.h> at the bottom of asm-x86_64/topology.h they get a
confusing mish-mash of pcibus functions.  What I mean is, on x86_64
pcibus_to_cpumask will do the right thing but pcibus_to_node will not.
This is broken.  The correct thing to do would be to define pcibus_to_node
in asm-x86_64/topology.h and only include asm-generic/topology.h if
DISCONTIG/NUMA isn't defined.

IA64 seems to define all the topo functions except pcibus related ones.
They also include the generic topology functions, but at least their
pcibus_to_FOO functions are consistent.  Ideally, IA64 would also implement
a version of pcibus_to_FOO that does the right thing, but for now I guess
it isn't a priority for them.

Either way, I suppose it's not a terribly interesting argument.  Both of
our patches solve the build problem.  Whichever one Andrew wants is the fix
we'll go with.  I'd obviously prefer mine, since it remains consistent with
the way I designed the topology system in the first place, but since 2
other arches do it the way your patch does, I won't throw a fit if your
patch gets in.

-Matt
-
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/