diff mbox series

[13/37] xen/x86: decouple processor_nodes_parsed from acpi numa functions

Message ID 20210923120236.3692135-14-wei.chen@arm.com (mailing list archive)
State New, archived
Headers show
Series Add device tree based NUMA support to Arm | expand

Commit Message

Wei Chen Sept. 23, 2021, 12:02 p.m. UTC
Xen is using processor_nodes_parsed to record parsed processor nodes
from ACPI table or other firmware provided resource table. This
variable is used in ACPI numa functions directly. In follow-up
patchs, neutral NUMA code will be abstracted and move to other files.
So in this patch, we introduce numa_set_processor_nodes_parsed helper
to decouple processor_nodes_parsed from acpi numa functions.

Signed-off-by: Wei Chen <wei.chen@arm.com>
---
 xen/arch/x86/srat.c        | 9 +++++++--
 xen/include/asm-x86/numa.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

Comments

Stefano Stabellini Sept. 24, 2021, 12:40 a.m. UTC | #1
+x86 maintainers


On Thu, 23 Sep 2021, Wei Chen wrote:
> Xen is using processor_nodes_parsed to record parsed processor nodes
> from ACPI table or other firmware provided resource table. This
> variable is used in ACPI numa functions directly. In follow-up
> patchs, neutral NUMA code will be abstracted and move to other files.
> So in this patch, we introduce numa_set_processor_nodes_parsed helper
> to decouple processor_nodes_parsed from acpi numa functions.
> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> ---
>  xen/arch/x86/srat.c        | 9 +++++++--
>  xen/include/asm-x86/numa.h | 1 +
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
> index aa07a7e975..9276a52138 100644
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -104,6 +104,11 @@ nodeid_t setup_node(unsigned pxm)
>  	return node;
>  }
>  
> +void  __init numa_set_processor_nodes_parsed(nodeid_t node)
> +{
> +	node_set(node, processor_nodes_parsed);
> +}
> +
>  bool __init numa_memblks_available(void)
>  {
>  	if (num_node_memblks < NR_NODE_MEMBLKS)
> @@ -236,7 +241,7 @@ acpi_numa_x2apic_affinity_init(const struct acpi_srat_x2apic_cpu_affinity *pa)
>  	}
>  
>  	apicid_to_node[pa->apic_id] = node;
> -	node_set(node, processor_nodes_parsed);
> +	numa_set_processor_nodes_parsed(node);
>  	acpi_numa = 1;
>  
>  	if (opt_acpi_verbose)
> @@ -271,7 +276,7 @@ acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
>  		return;
>  	}
>  	apicid_to_node[pa->apic_id] = node;
> -	node_set(node, processor_nodes_parsed);
> +	numa_set_processor_nodes_parsed(node);
>  	acpi_numa = 1;
>  
>  	if (opt_acpi_verbose)
> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> index 78e044a390..295f875a51 100644
> --- a/xen/include/asm-x86/numa.h
> +++ b/xen/include/asm-x86/numa.h
> @@ -77,6 +77,7 @@ extern int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node);
>  extern bool numa_memblks_available(void);
>  extern int numa_update_node_memblks(nodeid_t node,
>  		paddr_t start, paddr_t size, bool hotplug);
> +extern void numa_set_processor_nodes_parsed(nodeid_t node);
>  
>  void srat_parse_regions(paddr_t addr);
>  extern u8 __node_distance(nodeid_t a, nodeid_t b);
> -- 
> 2.25.1
>
Jan Beulich Jan. 25, 2022, 9:49 a.m. UTC | #2
On 23.09.2021 14:02, Wei Chen wrote:
> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -104,6 +104,11 @@ nodeid_t setup_node(unsigned pxm)
>  	return node;
>  }
>  
> +void  __init numa_set_processor_nodes_parsed(nodeid_t node)

Besides (nit) the stray blank here, earlier comments apply. The way you
do the rearrangement it is close to impossible to see the actual "why"
behind the changes. Even if it would make for a bigger patch, I think
you want to collapse patches and move things out of srat.c code while
you split out generically usable functionality. Or, if the result was
deemed to large a patch to have all in one go, make the movement of
individual static variables the topic of each patch, introducing
accessor functions like the one here.

Jan
Wei Chen Jan. 27, 2022, 8:06 a.m. UTC | #3
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年1月25日 17:49
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; xen-
> devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org
> Subject: Re: [PATCH 13/37] xen/x86: decouple processor_nodes_parsed from
> acpi numa functions
> 
> On 23.09.2021 14:02, Wei Chen wrote:
> > --- a/xen/arch/x86/srat.c
> > +++ b/xen/arch/x86/srat.c
> > @@ -104,6 +104,11 @@ nodeid_t setup_node(unsigned pxm)
> >  	return node;
> >  }
> >
> > +void  __init numa_set_processor_nodes_parsed(nodeid_t node)
> 
> Besides (nit) the stray blank here, earlier comments apply. The way you
> do the rearrangement it is close to impossible to see the actual "why"
> behind the changes. Even if it would make for a bigger patch, I think
> you want to collapse patches and move things out of srat.c code while
> you split out generically usable functionality. Or, if the result was
> deemed to large a patch to have all in one go, make the movement of
> individual static variables the topic of each patch, introducing
> accessor functions like the one here.
> 

Thanks, I will fix them in next version.

> Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index aa07a7e975..9276a52138 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -104,6 +104,11 @@  nodeid_t setup_node(unsigned pxm)
 	return node;
 }
 
+void  __init numa_set_processor_nodes_parsed(nodeid_t node)
+{
+	node_set(node, processor_nodes_parsed);
+}
+
 bool __init numa_memblks_available(void)
 {
 	if (num_node_memblks < NR_NODE_MEMBLKS)
@@ -236,7 +241,7 @@  acpi_numa_x2apic_affinity_init(const struct acpi_srat_x2apic_cpu_affinity *pa)
 	}
 
 	apicid_to_node[pa->apic_id] = node;
-	node_set(node, processor_nodes_parsed);
+	numa_set_processor_nodes_parsed(node);
 	acpi_numa = 1;
 
 	if (opt_acpi_verbose)
@@ -271,7 +276,7 @@  acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *pa)
 		return;
 	}
 	apicid_to_node[pa->apic_id] = node;
-	node_set(node, processor_nodes_parsed);
+	numa_set_processor_nodes_parsed(node);
 	acpi_numa = 1;
 
 	if (opt_acpi_verbose)
diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
index 78e044a390..295f875a51 100644
--- a/xen/include/asm-x86/numa.h
+++ b/xen/include/asm-x86/numa.h
@@ -77,6 +77,7 @@  extern int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node);
 extern bool numa_memblks_available(void);
 extern int numa_update_node_memblks(nodeid_t node,
 		paddr_t start, paddr_t size, bool hotplug);
+extern void numa_set_processor_nodes_parsed(nodeid_t node);
 
 void srat_parse_regions(paddr_t addr);
 extern u8 __node_distance(nodeid_t a, nodeid_t b);