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

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel