Re: [PATCH] powerpc/numa: Perform full re-add of CPU for PRRN/VPHN topology update

2018-11-14 Thread Michael Bringmann



On 11/13/2018 02:39 AM, Srikar Dronamraju wrote:
>> -static void topology_work_fn(struct work_struct *work)
>> -{
>> -rebuild_sched_domains();
>> +if (changed)
>> +rebuild_sched_domains();
>>  }
>>  static DECLARE_WORK(topology_work, topology_work_fn);
>>
>> @@ -1553,7 +1424,6 @@ void __init shared_proc_topology_init(void)
>>  if (lppaca_shared_proc(get_lppaca())) {
>>  bitmap_fill(cpumask_bits(_associativity_changes_mask),
>>  nr_cpumask_bits);
>> -numa_update_cpu_topology(false);
> 
> Shouldn't we be calling topology_schedule_update() here?

Agreed.

> 
>>  }
>>  }
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:   (512) 466-0650
m...@linux.vnet.ibm.com



Re: [PATCH] powerpc/numa: Perform full re-add of CPU for PRRN/VPHN topology update

2018-11-13 Thread Srikar Dronamraju
> -static void topology_work_fn(struct work_struct *work)
> -{
> - rebuild_sched_domains();
> + if (changed)
> + rebuild_sched_domains();
>  }
>  static DECLARE_WORK(topology_work, topology_work_fn);
> 
> @@ -1553,7 +1424,6 @@ void __init shared_proc_topology_init(void)
>   if (lppaca_shared_proc(get_lppaca())) {
>   bitmap_fill(cpumask_bits(_associativity_changes_mask),
>   nr_cpumask_bits);
> - numa_update_cpu_topology(false);

Shouldn't we be calling topology_schedule_update() here?

>   }
>  }
> 


-- 
Thanks and Regards
Srikar Dronamraju



[PATCH] powerpc/numa: Perform full re-add of CPU for PRRN/VPHN topology update

2018-11-12 Thread Michael Bringmann
On pseries systems, performing changes to a partition's affinity
can result in altering the nodes a CPU is assigned to the
current system.  For example, some systems are subject to resource
balancing operations by the operator or control software.  In such
environments, system CPUs may be in node 1 and 3 at boot, and be
moved to nodes 2, 3, and 5, for better performance.

The current implementation attempts to recognize such changes within
the powerpc-specific version of arch_update_cpu_topology to modify a
range of system data structures directly.  However, some scheduler
data structures may be inaccessible, or the timing of a node change
may still lead to corruption or error in other modules (e.g. user
space) which do not receive notification of these changes.

This patch modifies the PRRN/VPHN topology update worker function to
recognize an affinity change for a CPU, and to perform a full DLPAR
remove and add of the CPU instead of dynamically changing its node
to resolve this issue.

[Based upon patch submission:
Subject: [PATCH] powerpc/pseries: Perform full re-add of CPU for topology 
update post-migration
From: Nathan Fontenot 
Date: Tue Oct 30 05:43:36 AEDT 2018
]

[Replace patch submission:
Subject: [PATCH] powerpc/topology: Update numa mask when cpu node mapping 
changes
From: Srikar Dronamraju 
DAte: Wed Oct 10 15:24:46 AEDT 2018
]

Signed-off-by: Michael Bringmann 
---
 arch/powerpc/include/asm/topology.h |6 -
 arch/powerpc/kernel/rtasd.c |1 
 arch/powerpc/mm/numa.c  |  184 +--
 3 files changed, 27 insertions(+), 164 deletions(-)

diff --git a/arch/powerpc/include/asm/topology.h 
b/arch/powerpc/include/asm/topology.h
index f85e2b0..9f85246 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -42,7 +42,6 @@ static inline int pcibus_to_node(struct pci_bus *bus)
 
 extern int sysfs_add_device_to_node(struct device *dev, int nid);
 extern void sysfs_remove_device_from_node(struct device *dev, int nid);
-extern int numa_update_cpu_topology(bool cpus_locked);
 
 static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node)
 {
@@ -77,11 +76,6 @@ static inline void sysfs_remove_device_from_node(struct 
device *dev,
 {
 }
 
-static inline int numa_update_cpu_topology(bool cpus_locked)
-{
-   return 0;
-}
-
 static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node) {}
 
 #endif /* CONFIG_NUMA */
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 38cadae..c161d74 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -285,7 +285,6 @@ static void handle_prrn_event(s32 scope)
 * the RTAS event.
 */
pseries_devicetree_update(-scope);
-   numa_update_cpu_topology(false);
 }
 
 static void handle_rtas_event(const struct rtas_error_log *log)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index be6216e..f79b65f 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1236,96 +1236,25 @@ int find_and_online_cpu_nid(int cpu)
return new_nid;
 }
 
-/*
- * Update the CPU maps and sysfs entries for a single CPU when its NUMA
- * characteristics change. This function doesn't perform any locking and is
- * only safe to call from stop_machine().
- */
-static int update_cpu_topology(void *data)
-{
-   struct topology_update_data *update;
-   unsigned long cpu;
-
-   if (!data)
-   return -EINVAL;
-
-   cpu = smp_processor_id();
-
-   for (update = data; update; update = update->next) {
-   int new_nid = update->new_nid;
-   if (cpu != update->cpu)
-   continue;
-
-   unmap_cpu_from_node(cpu);
-   map_cpu_to_node(cpu, new_nid);
-   set_cpu_numa_node(cpu, new_nid);
-   set_cpu_numa_mem(cpu, local_memory_node(new_nid));
-   vdso_getcpu_init();
-   }
-
-   return 0;
-}
-
-static int update_lookup_table(void *data)
-{
-   struct topology_update_data *update;
-
-   if (!data)
-   return -EINVAL;
-
-   /*
-* Upon topology update, the numa-cpu lookup table needs to be updated
-* for all threads in the core, including offline CPUs, to ensure that
-* future hotplug operations respect the cpu-to-node associativity
-* properly.
-*/
-   for (update = data; update; update = update->next) {
-   int nid, base, j;
-
-   nid = update->new_nid;
-   base = cpu_first_thread_sibling(update->cpu);
-
-   for (j = 0; j < threads_per_core; j++) {
-   update_numa_cpu_lookup_table(base + j, nid);
-   }
-   }
-
-   return 0;
-}
-
-/*
- * Update the node maps and sysfs entries for each cpu whose home node
- * has changed. Returns 1 when the topology has changed, and 0 otherwise.
- *
- * cpus_locked says whether we