Re: [Xen-devel] [PATCH] x86/numa: Adjust datatypes for node and pxm

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.02.15 at 19:14, <boris.ostrovsky@xxxxxxxxxx> wrote:
> Use u8-sized node IDs and unsigned PXMs consistently throughout
> code (and introduce nodeid_t type).
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

I think the description should call out areas not covered, like the
node encoding in the top bits of MEMF_*.

> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -21,44 +21,55 @@
>  #include <asm/e820.h>
>  #include <asm/page.h>
>  
> +#define MAX_PXM   255

Perhaps better (MAX_NUMNODES - 1) than a literal number? Or
even do away with it altogether, use MAX_NUMNODES - 1 in the
array definition and ARRAY_SIZE() elsewhere?

> -__devinit int setup_node(int pxm)
> +__devinit nodeid_t setup_node(unsigned pxm)
>  {
> -     unsigned node = pxm2node[pxm];
> -     if (node == 0xff) {
> +     nodeid_t node;
> +
> +     /* NUMA_NO_NODE is 255 */
> +     BUILD_BUG_ON(MAX_NUMNODES > 254);

BUILD_BUG_ON(MAX_NUMNODES >= NUMA_NO_NODE);
(yielding the comment redundant)

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -581,7 +581,7 @@ static struct page_info *alloc_heap_pages(
>      struct domain *d)
>  {
>      unsigned int first_node, i, j, zone = 0, nodemask_retry = 0;
> -    unsigned int node = (uint8_t)((memflags >> _MEMF_node) - 1);
> +    nodeid_t node = (nodeid_t)((memflags >> _MEMF_node) - 1);

No-one will notice this needing adjustment if nodeid_t ever gets
widened. We'll need a _MEMF_node_width or _MEMF_node_mask
to do proper masking here (the latter would allow the use of
MASK_EXTR()), avoiding the need for a cast altogether.

Jan


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