This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH 1/6][RESEND] xen: Add NUMA support to Xen

On 12 May 2006, at 16:12, Ryan Harper wrote:

I don't want to add a compile option for this -- I want NUMA enabled
all the time and to add insignificant overhead on non-NUMA (or
small-NUMA like AMD64) systems. In fact, there's no reason for it to
add significant overhead in any situation really.

 -- Keir

Respun with no build-time option.

The obvious file to pick on in this patchset is arch/x86/numa.c, derived from Linux's srat.c, but I expect the points can be applied more generally: 1. You re-indented. Normally a good thing but not for copies of Linux source files. Please edit them and maintain them in Linux style (inc. hard tabs) as it makes it easier to sync with Linux updates. 2. Pointless EXPORT_SYMBOL() lines added. I can fully understand keeping existing ones to make the diff smaller, but *why* would you add new ones? 3. You rename variables: the variable 'cpu_affinity' in parse_cpu_affinity_structure() has become 'ca'. Now, I prefer your shorter name particularly since you add a bunch of code to that function, but it will make forward porting to new Linux source versions harder than it needs to be. Please don't do it.

Rather than taking Linux's srat.c and turning it into a more generic 'numa.c' file for Xen, perhaps it makes sense to define arch/x86/numa/ and stick srat.c in that subdir, and then add new file(s) in there for non-srat-related stuff?

 -- Keir

Xen-devel mailing list