Re: [Xen-devel] [RFC PATCH v3 08/24] NUMA: x86: Move numa code and make it generic

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 18/07/17 16:29, Wei Liu wrote:
> On Tue, Jul 18, 2017 at 05:11:30PM +0530, vijay.kilari@xxxxxxxxx wrote:
> [...]
> > diff --git a/xen/common/numa.c b/xen/common/numa.c
> > new file mode 100644
> > index 0000000..0381f1b
> > --- /dev/null
> > +++ b/xen/common/numa.c
> > @@ -0,0 +1,487 @@
> > +/*
> > + * Common NUMA handling functions for x86 and arm.
> > + * Original code extracted from arch/x86/numa.c
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms and conditions of the GNU General Public
> > + * License, version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <xen/init.h>
> > +#include <xen/ctype.h>
> > +#include <xen/sched.h>
> > +#include <xen/nodemask.h>
> > +#include <xen/numa.h>
> > +#include <xen/keyhandler.h>
> > +#include <xen/time.h>
> > +#include <xen/smp.h>
> > +#include <xen/pfn.h>
> > +#include <xen/mm.h>
> > +#include <xen/softirq.h>
> > +#include <xen/string.h>
> > +#include <asm/acpi.h>
> > +
> Since you're moving code anyway, please sort the headers alphabetically.
>
> > +static int numa_setup(char *s);
> > +custom_param("numa", numa_setup);
> > +
> > +struct node_data node_data[MAX_NUMNODES];
> > +
> > +/* Mapping from pdx to node id */
> Is this comment applicable to ARM? Does arm has PDX?
Yes ARM has PDX. For new architecture we expect the code to provide 
dummy helpers if they want to support NUMA.
> > +unsigned int memnode_shift;
> > +
> > +/*
> > + * In case of numa init failure or numa off,
> > + * memnode_shift is initialized to BITS_PER_LONG - 1. Hence allocate
> > + * memnodemap[] of BITS_PER_LONG.
> > + */
> > +static typeof(*memnodemap) _memnodemap[BITS_PER_LONG];
> > +unsigned long memnodemapsize;
> > +uint8_t *memnodemap;
> > +
> > +nodeid_t __read_mostly cpu_to_node[NR_CPUS] = {
> > +    [0 ... NR_CPUS-1] = NUMA_NO_NODE
> > +};
> > +
> > +cpumask_t __read_mostly node_to_cpumask[MAX_NUMNODES];
> > +
> > +bool numa_off;
> > +s8 acpi_numa = 0;
> > +
> > +int srat_disabled(void)
> bool here.
>
> Should probably be done in a previous patch.
Actually, the previous version had srat_disabled return bool. I am aware 
that Jan and I requested to keep acpi_numa as int, I didn't find any 
request of keep moving srat_disabled to int. So can you explain why??
> > +
> > +void __init numa_init_array(void)
> > +{
> > +    int rr, i;
> > +
> > +    /* There are unfortunately some poorly designed mainboards around
> > +       that only connect memory to a single CPU. This breaks the 1:1 cpu->node
> > +       mapping. To avoid this fill in the mapping for all possible
> > +       CPUs, as the number of CPUs is not known yet.
> > +       We round robin the existing nodes. */
> Please fix the coding style issue here.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel