[PATCH 0/5] arm64: numa/topology/smp: fix the cpumasks for CPU hotplug

[PATCH 0/5] arm64: numa/topology/smp: fix the cpumasks for CPU hotplug

[Sudeep Holla]

Hi Will, Catalin, Jeremy, Morten,

This is the fix I could come up for the issues we are seeing with arm64
for-next branch, in particular with commit 37c3ec2d810f ("arm64: topology:
divorce MC scheduling domain from core_siblings").

The solution is to update the CPU topology during CPU hotplug operations
similar to other architectures like x86 and PPC. This is also inline
with the expection from the scheduler.

I have cc-ed few Cavium and Huawei guys as they seem to have added or
modified the numa related code in the past.

Regards,
Sudeep

Sudeep Holla (5):
  arm64: topology: refactor reset_cpu_topology to add support for removing topology
  arm64: topology: add support to remove cpu topology
  arm64: numa: separate out updates to percpu nodeid and NUMA node cpumap
  arm64: smp: remove cpu and numa topology information when hotplugging out CPU
  arm64: topology: rename llc_siblings to align with other struct members

 arch/arm64/include/asm/numa.h     |  4 +++
 arch/arm64/include/asm/topology.h |  3 ++-
 arch/arm64/kernel/smp.c           |  5 ++++
 arch/arm64/kernel/topology.c      | 54 +++++++++++++++++++++++++++------------
 arch/arm64/mm/numa.c              | 29 +++++++++++++++------
 5 files changed, 70 insertions(+), 25 deletions(-)

--
2.7.4

[PATCH 1/5] arm64: topology: refactor reset_cpu_topology to add support for removing topology

[Sudeep Holla]

Currently reset_cpu_topology clears all the CPU topology information
and resets to default values. However we may need to just clear the
information when we hotplig out the CPU. In preparation to add the
support the same, let's refactor reset_cpu_topology to clear out the
information and reset them only if explicitly requested.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kernel/topology.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7415c166281f..f10babcc112b 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -296,28 +296,35 @@ void store_cpu_topology(unsigned int cpuid)
 	update_siblings_masks(cpuid);
 }
 
-static void __init reset_cpu_topology(void)
+static void clear_cpu_topology(int cpu, bool reset)
 {
-	unsigned int cpu;
+	struct cpu_topology *cpu_topo = &cpu_topology[cpu];
 
-	for_each_possible_cpu(cpu) {
-		struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+	cpu_topo->core_id = -1;
+	cpu_topo->thread_id = -1;
+	cpu_topo->package_id = -1;
+	cpu_topo->llc_id = -1;
 
-		cpu_topo->thread_id = -1;
-		cpu_topo->core_id = 0;
-		cpu_topo->package_id = -1;
+	cpumask_clear(&cpu_topo->llc_siblings);
+	cpumask_clear(&cpu_topo->core_sibling);
+	cpumask_clear(&cpu_topo->thread_sibling);
 
-		cpu_topo->llc_id = -1;
-		cpumask_clear(&cpu_topo->llc_siblings);
+	if (reset) {
+		cpu_topo->core_id = 0;
 		cpumask_set_cpu(cpu, &cpu_topo->llc_siblings);
-
-		cpumask_clear(&cpu_topo->core_sibling);
 		cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
-		cpumask_clear(&cpu_topo->thread_sibling);
 		cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
 	}
 }
 
+static void __init reset_cpu_topology(void)
+{
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu)
+		clear_cpu_topology(cpu, true);
+}
+
 #ifdef CONFIG_ACPI
 /*
  * Propagate the topology information of the processor_topology_node tree to the
-- 
2.7.4

[PATCH 1/5] arm64: topology: refactor reset_cpu_topology to add support for removing topology

[Jeffrey Hugo]

On 6/4/2018 4:39 AM, Sudeep Holla wrote:
> Currently reset_cpu_topology clears all the CPU topology information
> and resets to default values. However we may need to just clear the
> information when we hotplig out the CPU. In preparation to add the

hotplug

> support the same, let's refactor reset_cpu_topology to clear out the
> information and reset them only if explicitly requested.
> 



-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 2/5] arm64: topology: add support to remove cpu topology

[Sudeep Holla]

This patch adds support to remove all the CPU topology information using
clear_cpu_topology and also resetting the sibling information on other
sibling CPUs. This will be used in cpu_disable so that all the topology
information is removed on CPU hotplug out.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/topology.h |  1 +
 arch/arm64/kernel/topology.c      | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index df48212f767b..fb996f454305 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -23,6 +23,7 @@ extern struct cpu_topology cpu_topology[NR_CPUS];
 
 void init_cpu_topology(void);
 void store_cpu_topology(unsigned int cpuid);
+void remove_cpu_topology(unsigned int cpuid);
 const struct cpumask *cpu_coregroup_mask(int cpu);
 
 #ifdef CONFIG_NUMA
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index f10babcc112b..bf522bcf40ec 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -325,6 +325,21 @@ static void __init reset_cpu_topology(void)
 		clear_cpu_topology(cpu, true);
 }
 
+#define cpu_llc_shared_mask(cpu)	(&cpu_topology[cpu].llc_siblings)
+void remove_cpu_topology(unsigned int cpu)
+{
+	int sibling;
+
+	for_each_cpu(sibling, topology_core_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
+	for_each_cpu(sibling, topology_sibling_cpumask(cpu))
+		cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
+	for_each_cpu(sibling, cpu_llc_shared_mask(cpu))
+		cpumask_clear_cpu(cpu, cpu_llc_shared_mask(sibling));
+
+	clear_cpu_topology(cpu, false);
+}
+
 #ifdef CONFIG_ACPI
 /*
  * Propagate the topology information of the processor_topology_node tree to the
-- 
2.7.4

[PATCH 3/5] arm64: numa: separate out updates to percpu nodeid and NUMA node cpumap

[Sudeep Holla]

Currently numa_clear_node removes both cpu information from the NUMA
node cpumap as well as the NUMA node id from the cpu. Similarly
numa_store_cpu_info updates both percpu nodeid and NUMA cpumap.

However we need to retain the numa node id for the cpu and only remove
the cpu information from the numa node cpumap during CPU hotplug out.
The same can be extended for hotplugging in the CPU.

This patch separates out numa_{add,remove}_cpu from numa_clear_node and
numa_store_cpu_info.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/numa.h |  4 ++++
 arch/arm64/kernel/smp.c       |  2 ++
 arch/arm64/mm/numa.c          | 29 +++++++++++++++++++++--------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
index 01bc46d5b43a..626ad01e83bf 100644
--- a/arch/arm64/include/asm/numa.h
+++ b/arch/arm64/include/asm/numa.h
@@ -35,10 +35,14 @@ void __init numa_set_distance(int from, int to, int distance);
 void __init numa_free_distance(void);
 void __init early_map_cpu_to_node(unsigned int cpu, int nid);
 void numa_store_cpu_info(unsigned int cpu);
+void numa_add_cpu(unsigned int cpu);
+void numa_remove_cpu(unsigned int cpu);
 
 #else	/* CONFIG_NUMA */
 
 static inline void numa_store_cpu_info(unsigned int cpu) { }
+static inline void numa_add_cpu(unsigned int cpu) { }
+static inline void numa_remove_cpu(unsigned int cpu) { }
 static inline void arm64_numa_init(void) { }
 static inline void early_map_cpu_to_node(unsigned int cpu, int nid) { }
 
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index f3e2e3aec0b0..49a021e30dfb 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -225,6 +225,7 @@ asmlinkage void secondary_start_kernel(void)
 	notify_cpu_starting(cpu);
 
 	store_cpu_topology(cpu);
+	numa_add_cpu(cpu);
 
 	/*
 	 * OK, now it's safe to let the boot CPU continue.  Wait for
@@ -679,6 +680,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	this_cpu = smp_processor_id();
 	store_cpu_topology(this_cpu);
 	numa_store_cpu_info(this_cpu);
+	numa_add_cpu(this_cpu);
 
 	/*
 	 * If UP is mandated by "nosmp" (which implies "maxcpus=0"), don't set
diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index dad128ba98bf..43cc669bc7bc 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -70,19 +70,32 @@ EXPORT_SYMBOL(cpumask_of_node);
 
 #endif
 
-static void map_cpu_to_node(unsigned int cpu, int nid)
+static void numa_update_cpu(unsigned int cpu, bool remove)
 {
-	set_cpu_numa_node(cpu, nid);
-	if (nid >= 0)
+	int nid = cpu_to_node(cpu);
+
+	if (nid < 0)
+		return;
+
+	if (remove)
+		cpumask_clear_cpu(cpu, node_to_cpumask_map[nid]);
+	else
 		cpumask_set_cpu(cpu, node_to_cpumask_map[nid]);
 }
 
-void numa_clear_node(unsigned int cpu)
+void numa_add_cpu(unsigned int cpu)
 {
-	int nid = cpu_to_node(cpu);
+	numa_update_cpu(cpu, false);
+}
 
-	if (nid >= 0)
-		cpumask_clear_cpu(cpu, node_to_cpumask_map[nid]);
+void numa_remove_cpu(unsigned int cpu)
+{
+	numa_update_cpu(cpu, true);
+}
+
+void numa_clear_node(unsigned int cpu)
+{
+	numa_remove_cpu(cpu);
 	set_cpu_numa_node(cpu, NUMA_NO_NODE);
 }
 
@@ -116,7 +129,7 @@ static void __init setup_node_to_cpumask_map(void)
  */
 void numa_store_cpu_info(unsigned int cpu)
 {
-	map_cpu_to_node(cpu, cpu_to_node_map[cpu]);
+	set_cpu_numa_node(cpu, cpu_to_node_map[cpu]);
 }
 
 void __init early_map_cpu_to_node(unsigned int cpu, int nid)
-- 
2.7.4

[PATCH 4/5] arm64: smp: remove cpu and numa topology information when hotplugging out CPU

[Sudeep Holla]

We already repopulate the information on CPU hotplug-in, so we can safely
remove the CPU topology and NUMA cpumap information during CPU hotplug
out operation. This will help to provide the correct cpumask for
scheduler domains.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kernel/smp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 49a021e30dfb..63a40ba3cd37 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -279,6 +279,9 @@ int __cpu_disable(void)
 	if (ret)
 		return ret;
 
+	remove_cpu_topology(cpu);
+	numa_remove_cpu(cpu);
+
 	/*
 	 * Take this CPU offline.  Once we clear this, we can't return,
 	 * and we must not schedule until we're ready to give up the cpu.
-- 
2.7.4

[PATCH 5/5] arm64: topology: rename llc_siblings to align with other struct members

[Sudeep Holla]

Similar to core_sibling and thread_sibling, it's better to align and
rename llc_siblings to llc_sibling.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/topology.h |  2 +-
 arch/arm64/kernel/topology.c      | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index fb996f454305..dda4b6dba6b4 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -11,7 +11,7 @@ struct cpu_topology {
 	int llc_id;
 	cpumask_t thread_sibling;
 	cpumask_t core_sibling;
-	cpumask_t llc_siblings;
+	cpumask_t llc_sibling;
 };
 
 extern struct cpu_topology cpu_topology[NR_CPUS];
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index bf522bcf40ec..082f76f7a94d 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -223,8 +223,8 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
 		core_mask = &cpu_topology[cpu].core_sibling;
 	}
 	if (cpu_topology[cpu].llc_id != -1) {
-		if (cpumask_subset(&cpu_topology[cpu].llc_siblings, core_mask))
-			core_mask = &cpu_topology[cpu].llc_siblings;
+		if (cpumask_subset(&cpu_topology[cpu].llc_sibling, core_mask))
+			core_mask = &cpu_topology[cpu].llc_sibling;
 	}
 
 	return core_mask;
@@ -240,7 +240,7 @@ static void update_siblings_masks(unsigned int cpuid)
 		cpu_topo = &cpu_topology[cpu];
 
 		if (cpuid_topo->llc_id == cpu_topo->llc_id)
-			cpumask_set_cpu(cpu, &cpuid_topo->llc_siblings);
+			cpumask_set_cpu(cpu, &cpuid_topo->llc_sibling);
 
 		if (cpuid_topo->package_id != cpu_topo->package_id)
 			continue;
@@ -305,13 +305,13 @@ static void clear_cpu_topology(int cpu, bool reset)
 	cpu_topo->package_id = -1;
 	cpu_topo->llc_id = -1;
 
-	cpumask_clear(&cpu_topo->llc_siblings);
+	cpumask_clear(&cpu_topo->llc_sibling);
 	cpumask_clear(&cpu_topo->core_sibling);
 	cpumask_clear(&cpu_topo->thread_sibling);
 
 	if (reset) {
 		cpu_topo->core_id = 0;
-		cpumask_set_cpu(cpu, &cpu_topo->llc_siblings);
+		cpumask_set_cpu(cpu, &cpu_topo->llc_sibling);
 		cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
 		cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
 	}
@@ -325,7 +325,7 @@ static void __init reset_cpu_topology(void)
 		clear_cpu_topology(cpu, true);
 }
 
-#define cpu_llc_shared_mask(cpu)	(&cpu_topology[cpu].llc_siblings)
+#define cpu_llc_shared_mask(cpu)	(&cpu_topology[cpu].llc_sibling)
 void remove_cpu_topology(unsigned int cpu)
 {
 	int sibling;
-- 
2.7.4

[PATCH 0/5] arm64: numa/topology/smp: fix the cpumasks for CPU hotplug

[Hanjun Guo]

Hi Sudeep,

On 2018/6/4 18:39, Sudeep Holla wrote:
> Hi Will, Catalin, Jeremy, Morten,
> 
> This is the fix I could come up for the issues we are seeing with arm64
> for-next branch, in particular with commit 37c3ec2d810f ("arm64: topology:
> divorce MC scheduling domain from core_siblings").
> 
> The solution is to update the CPU topology during CPU hotplug operations
> similar to other architectures like x86 and PPC. This is also inline
> with the expection from the scheduler.
> 
> I have cc-ed few Cavium and Huawei guys as they seem to have added or
> modified the numa related code in the past.

Sorry for the late response, I can test on ARM64 NUMA systems
with PPTT enabled, before I'm doing that, could you give some
suggestion of the testing? what specific test case should I
test? doing CPU offline/online when PPTT is enabled?

Thanks
Hanjun

[PATCH 0/5] arm64: numa/topology/smp: fix the cpumasks for CPU hotplug

[Sudeep Holla]

On 12/06/18 02:14, Hanjun Guo wrote:
> Hi Sudeep,
> 
> On 2018/6/4 18:39, Sudeep Holla wrote:
>> Hi Will, Catalin, Jeremy, Morten,
>>
>> This is the fix I could come up for the issues we are seeing with arm64
>> for-next branch, in particular with commit 37c3ec2d810f ("arm64: topology:
>> divorce MC scheduling domain from core_siblings").
>>
>> The solution is to update the CPU topology during CPU hotplug operations
>> similar to other architectures like x86 and PPC. This is also inline
>> with the expection from the scheduler.
>>
>> I have cc-ed few Cavium and Huawei guys as they seem to have added or
>> modified the numa related code in the past.
> 
> Sorry for the late response, I can test on ARM64 NUMA systems
> with PPTT enabled, before I'm doing that, could you give some
> suggestion of the testing? what specific test case should I
> test? doing CPU offline/online when PPTT is enabled?
> 

Thanks for the response. I plan to post v2 after -rc1 which will have
some delta from this. So don't test this for now. Yes CPU hotplug tests
will be good. Also some NUMA based(numactl) assignments and try
hotplugging all CPUs in a node and similar tests with PPTT would be good.

-- 
Regards,
Sudeep