Re: [PATCH v5] NUMA: Introduce NODE_DATA->node_present_pages(RAM pages)

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.11.2024 12:14, Bernhard Kaindl wrote:
> --- a/xen/common/numa.c
> +++ b/xen/common/numa.c
> @@ -4,6 +4,7 @@
>   * Adapted for Xen: Ryan Harper <ryanh@xxxxxxxxxx>
>   */
>  
> +#include "xen/pfn.h"
>  #include <xen/init.h>
>  #include <xen/keyhandler.h>
>  #include <xen/mm.h>

Please retain sorting, and please don't use ""-style includes when all others
use <>-style (and there's no specific reason for using quotes).

> @@ -499,15 +500,41 @@ int __init compute_hash_shift(const struct node *nodes,
>      return shift;
>  }
>  
> -/* Initialize NODE_DATA given nodeid and start/end */
> +/**
> + * @brief Initialize a NUMA node's node_data structure at boot.
> + *
> + * It is given the NUMA node's index in the node_data array as well
> + * as the start and exclusive end address of the node's memory span
> + * as arguments and initializes the node_data entry with this information.
> + *
> + * It then initializes the total number of usable memory pages within
> + * the NUMA node's memory span using the arch_get_ram_range() function.
> + *
> + * @param nodeid The index into the node_data array for the node.
> + * @param start The starting physical address of the node's memory range.
> + * @param end The exclusive ending physical address of the node's memory 
> range.
> + */
>  void __init setup_node_bootmem(nodeid_t nodeid, paddr_t start, paddr_t end)
>  {
>      unsigned long start_pfn = paddr_to_pfn(start);
>      unsigned long end_pfn = paddr_to_pfn(end);
> -
> -    NODE_DATA(nodeid)->node_start_pfn = start_pfn;
> -    NODE_DATA(nodeid)->node_spanned_pages = end_pfn - start_pfn;
> -
> +    struct node_data *numa_node = NODE_DATA(nodeid);

Nit: Just "node" as a name would likely suffice, without losing anything,
yet helping readability?

> +    unsigned int idx = 0;
> +    int err;
> +
> +    numa_node->node_start_pfn = start_pfn;
> +    numa_node->node_spanned_pages = end_pfn - start_pfn;
> +    numa_node->node_present_pages = 0;
> +
> +    /* Calculate the number of present RAM pages within the node */
> +    do {
> +        paddr_t ram_start, ram_end;
> +
> +        err = arch_get_ram_range(idx++, &ram_start, &ram_end);
> +        if ( !err && ram_start < end && ram_end > start )
> +            numa_node->node_present_pages += PFN_DOWN(min(ram_end, end)) -
> +                                             PFN_UP(max(ram_start, start));
> +    } while ( err != -ENOENT );
>      node_set_online(nodeid);

Nit: Blank line above here please.

With those (happy to take care of while committing):
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan