Re: [Xen-devel] [PATCH v2 1/3] x86/numa: Allow arbitrary value of PXM in PXM<->node mapping

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.02.15 at 20:11, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -27,35 +27,79 @@ static nodemask_t memory_nodes_parsed __initdata;
>  static nodemask_t processor_nodes_parsed __initdata;
>  static nodemask_t nodes_found __initdata;
>  static struct node nodes[MAX_NUMNODES] __initdata;
> -static u8 __read_mostly pxm2node[256] = { [0 ... 255] = NUMA_NO_NODE };
>  
> +struct pxm2node {
> +    unsigned pxm;
> +    int node;
> +};
> +static struct pxm2node __read_mostly p2n[MAX_NUMNODES] =

Can this please continue to be named pxm2node[]? "p2..." is too
commonly used for representing a phys-to-something conversion.

> +    { [0 ... MAX_NUMNODES - 1] = {.node = NUMA_NO_NODE} };
> +
> +static int node_to_pxm(int n);

I don't see why you need to reinstate this forward declaration.

>  int pxm_to_node(int pxm)
>  {
> -     if ((unsigned)pxm >= 256)
> -             return -1;
> -     /* Extend 0xff to (int)-1 */
> -     return (signed char)pxm2node[pxm];
> +    unsigned i;
> +
> +    if ( (pxm < MAX_NUMNODES) && node_found(pxm, pxm) )
> +        return p2n[pxm].node;

With this the function parameter's type can't remain to be signed.
Also please don't change coding style here, unless you do so for
the whole file (in a separate patch). The one change I wouldn't
mind is switching node_to_pxm() from 8-space to tab indentation.

>  __devinit int setup_node(int pxm)
>  {
> -     unsigned node = pxm2node[pxm];
> -     if (node == 0xff) {
> -             if (nodes_weight(nodes_found) >= MAX_NUMNODES)
> -                     return -1;
> -             node = first_unset_node(nodes_found); 
> -             node_set(node, nodes_found);
> -             pxm2node[pxm] = node;
> -     }
> -     return pxm2node[pxm];
> +    int node;
> +    unsigned idx;
> +    static bool_t warned;
> +
> +    if ( pxm < MAX_NUMNODES )
> +    {
> +        if ( node_found(pxm, pxm) )
> +            return p2n[pxm].node;
> +
> +        /* Try to maintain indexing of p2n by pxm */

Coding style.

> @@ -111,8 +155,6 @@ static __init void bad_srat(void)
>       acpi_numa = -1;
>       for (i = 0; i < MAX_LOCAL_APIC; i++)
>               apicid_to_node[i] = NUMA_NO_NODE;
> -     for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
> -             pxm2node[i] = NUMA_NO_NODE;

Without any replacement?

Jan


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