Re: [PATCH 2/2] NUMA: replace phys_to_nid()

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 13/12/2022 11:38 am, Jan Beulich wrote:
> All callers convert frame numbers (perhaps in turn derived from struct
> page_info pointers) to an address, just for the function to convert it
> back to a frame number (as the first step of paddr_to_pdx()). Replace
> the function by mfn_to_nid() plus a page_to_nid() wrapper macro. Replace
> call sites by the respectively most suitable one.
>
> While there also introduce a !NUMA stub, eliminating the need for Arm
> (and potentially other ports) to carry one individually.

Thanks.  This will help RISC-V too.

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, albeit with one
deletion.

> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -1,6 +1,7 @@
>  #ifndef _XEN_NUMA_H
>  #define _XEN_NUMA_H
>  
> +#include <xen/mm-frame.h>
>  #include <asm/numa.h>
>  
>  #define NUMA_NO_NODE     0xFF
> @@ -68,12 +69,15 @@ struct node_data {
>  
>  extern struct node_data node_data[];
>  
> -static inline nodeid_t __attribute_pure__ phys_to_nid(paddr_t addr)
> +static inline nodeid_t __attribute_pure__ mfn_to_nid(mfn_t mfn)
>  {
>      nodeid_t nid;
> -    ASSERT((paddr_to_pdx(addr) >> memnode_shift) < memnodemapsize);
> -    nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
> +    unsigned long pdx = mfn_to_pdx(mfn);
> +
> +    ASSERT((pdx >> memnode_shift) < memnodemapsize);
> +    nid = memnodemap[pdx >> memnode_shift];
>      ASSERT(nid < MAX_NUMNODES && node_data[nid].node_spanned_pages);
> +
>      return nid;
>  }
>  
> @@ -102,6 +106,15 @@ extern bool numa_update_node_memblks(nod
>                                       paddr_t start, paddr_t size, bool 
> hotplug);
>  extern void numa_set_processor_nodes_parsed(nodeid_t node);
>  
> +#else
> +
> +static inline nodeid_t __attribute_pure__ mfn_to_nid(mfn_t mfn)
> +{
> +    return 0;
> +}

pure is useless on a stub like this, whereas its false on the non-stub
form (uses several non-const variables) in a way that the compiler can
prove (because it's static inline), and will discard.

As you're modifying both lines anyway, just drop the attribute.

~Andrew