[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC PATCH v2 04/25] x86: NUMA: Add accessors for acpi_numa, numa_off and numa_fake variables
- To: vijay.kilari@xxxxxxxxx, xen-devel@xxxxxxxxxxxxx
- From: Julien Grall <julien.grall@xxxxxxx>
- Date: Thu, 20 Apr 2017 16:59:48 +0100
- Cc: sstabellini@xxxxxxxxxx, wei.liu2@xxxxxxxxxx, George.Dunlap@xxxxxxxxxxxxx, andrew.cooper3@xxxxxxxxxx, ian.jackson@xxxxxxxxxxxxx, tim@xxxxxxx, jbeulich@xxxxxxxx, Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
- Delivery-date: Thu, 20 Apr 2017 15:59:59 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
Hi Vijay,
On 28/03/17 16:53, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
Add accessor functions for acpi_numa and numa_off static
variables. Init value of acpi_numa is set 1 instead of 0.
Please explain why you change the acpi_numa value from 0 to 1.
Also, I am not sure to understand the benefits of those helpers. Why do
you need them? Why not using the global variable directly, this will
avoid to call a function just for returning a value...
Also return value of srat_disabled is changed to bool.
Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
---
xen/arch/x86/numa.c | 43 +++++++++++++++++++++++++++++++------------
xen/arch/x86/setup.c | 2 +-
xen/arch/x86/srat.c | 12 ++++++------
xen/include/asm-x86/acpi.h | 1 -
xen/include/asm-x86/numa.h | 5 +----
xen/include/xen/numa.h | 3 +++
6 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 964fc5a..6b794a7 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -42,12 +42,27 @@ cpumask_t __read_mostly node_to_cpumask[MAX_NUMNODES];
nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
-bool numa_off = 0;
-s8 acpi_numa = 0;
+static bool numa_off = 0;
+static bool acpi_numa = 1;
Please don't mix 0/1 and bool. Instead use false/true.
-int srat_disabled(void)
+bool is_numa_off(void)
{
- return numa_off || acpi_numa < 0;
+ return numa_off;
+}
+
+bool get_acpi_numa(void)
+{
+ return acpi_numa;
+}
+
+void set_acpi_numa(bool_t val)
+{
+ acpi_numa = val;
+}
+
+bool srat_disabled(void)
+{
+ return numa_off || acpi_numa == 0;
}
/*
@@ -202,13 +217,17 @@ void __init numa_init_array(void)
#ifdef CONFIG_NUMA_EMU
static int __initdata numa_fake = 0;
+static int get_numa_fake(void)
+{
+ return numa_fake;
+}
/* Numa emulation */
static int __init numa_emulation(uint64_t start_pfn, uint64_t end_pfn)
{
int i;
struct node nodes[MAX_NUMNODES];
- uint64_t sz = ((end_pfn - start_pfn) << PAGE_SHIFT) / numa_fake;
+ uint64_t sz = ((end_pfn - start_pfn) << PAGE_SHIFT) / get_numa_fake();
/* Kludge needed for the hash function */
if ( hweight64(sz) > 1 )
@@ -223,10 +242,10 @@ static int __init numa_emulation(uint64_t start_pfn,
uint64_t end_pfn)
}
memset(&nodes,0,sizeof(nodes));
- for ( i = 0; i < numa_fake; i++ )
+ for ( i = 0; i < get_numa_fake(); i++ )
{
nodes[i].start = (start_pfn << PAGE_SHIFT) + i * sz;
- if ( i == numa_fake - 1 )
+ if ( i == get_numa_fake() - 1 )
sz = (end_pfn << PAGE_SHIFT) - nodes[i].start;
nodes[i].end = nodes[i].start + sz;
printk(KERN_INFO
@@ -235,7 +254,7 @@ static int __init numa_emulation(uint64_t start_pfn,
uint64_t end_pfn)
(nodes[i].end - nodes[i].start) >> 20);
node_set_online(i);
}
- if ( compute_memnode_shift(nodes, numa_fake, NULL, &memnode_shift) )
+ if ( compute_memnode_shift(nodes, get_numa_fake(), NULL, &memnode_shift) )
{
memnode_shift = 0;
printk(KERN_ERR "No NUMA hash function found. Emulation disabled.\n");
@@ -254,18 +273,18 @@ void __init numa_initmem_init(unsigned long start_pfn,
unsigned long end_pfn)
int i;
#ifdef CONFIG_NUMA_EMU
- if ( numa_fake && !numa_emulation(start_pfn, end_pfn) )
+ if ( get_numa_fake() && !numa_emulation(start_pfn, end_pfn) )
return;
#endif
#ifdef CONFIG_ACPI_NUMA
- if ( !numa_off && !acpi_scan_nodes((uint64_t)start_pfn << PAGE_SHIFT,
+ if ( !is_numa_off() && !acpi_scan_nodes((uint64_t)start_pfn << PAGE_SHIFT,
(uint64_t)end_pfn << PAGE_SHIFT) )
return;
#endif
printk(KERN_INFO "%s\n",
- numa_off ? "NUMA turned off" : "No NUMA configuration found");
+ is_numa_off() ? "NUMA turned off" : "No NUMA configuration found");
printk(KERN_INFO "Faking a node at %016"PRIx64"-%016"PRIx64"\n",
(uint64_t)start_pfn << PAGE_SHIFT,
@@ -312,7 +331,7 @@ static int __init numa_setup(char *opt)
if ( !strncmp(opt,"noacpi",6) )
{
numa_off = 0;
- acpi_numa = -1;
+ acpi_numa = 0;
}
#endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 1cd290e..4410e53 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -241,7 +241,7 @@ void srat_detect_node(int cpu)
node_set_online(node);
numa_set_node(cpu, node);
- if ( opt_cpu_info && acpi_numa > 0 )
+ if ( opt_cpu_info && get_acpi_numa() )
printk("CPU %d APIC %d -> Node %d\n", cpu, apicid, node);
}
diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index 2d0c047..ccacbcd 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -153,7 +153,7 @@ static void __init bad_srat(void)
{
int i;
printk(KERN_ERR "SRAT: SRAT not used.\n");
- acpi_numa = -1;
+ set_acpi_numa(0);
for (i = 0; i < MAX_LOCAL_APIC; i++)
apicid_to_node[i] = NUMA_NO_NODE;
for (i = 0; i < ARRAY_SIZE(pxm2node); i++)
@@ -232,7 +232,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);
- acpi_numa = 1;
+ set_acpi_numa(1);
printk(KERN_INFO "SRAT: PXM %u -> APIC %08x -> Node %u\n",
pxm, pa->apic_id, node);
}
@@ -265,7 +265,7 @@ acpi_numa_processor_affinity_init(const struct
acpi_srat_cpu_affinity *pa)
}
apicid_to_node[pa->apic_id] = node;
node_set(node, processor_nodes_parsed);
- acpi_numa = 1;
+ set_acpi_numa(1);
printk(KERN_INFO "SRAT: PXM %u -> APIC %02x -> Node %u\n",
pxm, pa->apic_id, node);
}
@@ -418,7 +418,7 @@ static int __init srat_parse_region(struct
acpi_subtable_header *header,
(ma->flags & ACPI_SRAT_MEM_NON_VOLATILE))
return 0;
- if (numa_off)
+ if (is_numa_off())
printk(KERN_INFO "SRAT: %013"PRIx64"-%013"PRIx64"\n",
ma->base_address, ma->base_address + ma->length - 1);
@@ -433,7 +433,7 @@ void __init srat_parse_regions(uint64_t addr)
uint64_t mask;
unsigned int i;
- if (acpi_disabled || acpi_numa < 0 ||
+ if (acpi_disabled || (get_acpi_numa() == 0) ||
acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat))
return;
@@ -462,7 +462,7 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end)
for (i = 0; i < MAX_NUMNODES; i++)
cutoff_node(i, start, end);
- if (acpi_numa <= 0)
+ if (get_acpi_numa() == 0)
return -1;
if (!nodes_cover_memory()) {
diff --git a/xen/include/asm-x86/acpi.h b/xen/include/asm-x86/acpi.h
index a766688..9298d42 100644
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -103,7 +103,6 @@ extern void acpi_reserve_bootmem(void);
#define ARCH_HAS_POWER_INIT 1
-extern s8 acpi_numa;
extern int acpi_scan_nodes(u64 start, u64 end);
#define NR_NODE_MEMBLKS (MAX_NUMNODES*2)
diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
index bb22bff..ae5768b 100644
--- a/xen/include/asm-x86/numa.h
+++ b/xen/include/asm-x86/numa.h
@@ -30,10 +30,7 @@ extern nodeid_t pxm_to_node(unsigned int pxm);
extern void numa_add_cpu(int cpu);
extern void numa_init_array(void);
-extern bool_t numa_off;
-
-
-extern int srat_disabled(void);
+extern bool srat_disabled(void);
extern void numa_set_node(int cpu, nodeid_t node);
extern nodeid_t setup_node(unsigned int pxm);
extern void srat_detect_node(int cpu);
diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
index 7aef1a8..7f6d090 100644
--- a/xen/include/xen/numa.h
+++ b/xen/include/xen/numa.h
@@ -18,4 +18,7 @@
(((d)->vcpu != NULL && (d)->vcpu[0] != NULL) \
? vcpu_to_node((d)->vcpu[0]) : NUMA_NO_NODE)
+bool is_numa_off(void);
+bool get_acpi_numa(void);
+void set_acpi_numa(bool val);
I am not sure to understand why you add those helpers directly here in
xen/numa.h. IHMO, This should belong to the moving code patches.
#endif /* _XEN_NUMA_H */
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
