On 05/25/2017 10:35 AM, Michael Bringmann wrote:
>
> powerpc/numa: Correct the currently broken capability to set the
> topology for shared CPUs in LPARs. At boot time for shared CPU
> lpars, the topology for each shared CPU is set to node zero, however,
> this is now updated correctly using the Virtual Processor Home Node
> (VPHN) capabilities information provided by the pHyp. The VPHN handling
> in Linux is disabled, if PRRN handling is present.
>
> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 19 ++++++++++++++++++-
> arch/powerpc/platforms/pseries/dlpar.c | 2 ++
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 ++-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 371792e..15c2dd5 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -29,6 +29,7 @@
> #include <linux/seq_file.h>
> #include <linux/uaccess.h>
> #include <linux/slab.h>
> +#include <linux/sched.h>
> #include <asm/cputhreads.h>
> #include <asm/sparsemem.h>
> #include <asm/prom.h>
> @@ -42,6 +43,8 @@
> #include <asm/vdso.h>
>
> static int numa_enabled = 1;
> +static int topology_inited;
> +static int topology_update_needed;
>
> static char *cmdline __initdata;
>
> @@ -1321,8 +1324,11 @@ int arch_update_cpu_topology(void)
> struct device *dev;
> int weight, new_nid, i = 0;
>
> - if (!prrn_enabled && !vphn_enabled)
> + if (!prrn_enabled && !vphn_enabled) {
> + if (!topology_inited)
> + topology_update_needed = 1;
> return 0;
> + }
>
> weight = cpumask_weight(&cpu_associativity_changes_mask);
> if (!weight)
> @@ -1361,6 +1367,8 @@ int arch_update_cpu_topology(void)
> cpumask_andnot(&cpu_associativity_changes_mask,
> &cpu_associativity_changes_mask,
> cpu_sibling_mask(cpu));
> + pr_info("Assoc chg gives same node %d for cpu%d\n",
> + new_nid, cpu);
> cpu = cpu_last_thread_sibling(cpu);
> continue;
> }
> @@ -1377,6 +1385,9 @@ int arch_update_cpu_topology(void)
> cpu = cpu_last_thread_sibling(cpu);
> }
>
> + if (i)
> + updates[i-1].next = NULL;
> +
> pr_debug("Topology update for the following CPUs:\n");
> if (cpumask_weight(&updated_cpus)) {
> for (ud = &updates[0]; ud; ud = ud->next) {
> @@ -1423,6 +1434,7 @@ int arch_update_cpu_topology(void)
>
> out:
> kfree(updates);
> + topology_update_needed = 0;
> return changed;
> }
>
> @@ -1600,6 +1612,11 @@ static int topology_update_init(void)
> if (!proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops))
> return -ENOMEM;
>
> + topology_inited = 1;
> + if (topology_update_needed)
> + bitmap_fill(cpumask_bits(&cpu_associativity_changes_mask),
> + nr_cpumask_bits);
> +
> return 0;
> }
> device_initcall(topology_update_init);
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c
> b/arch/powerpc/platforms/pseries/dlpar.c
> index bda18d8..5106263 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -592,6 +592,8 @@ static ssize_t dlpar_show(struct class *class, struct
> class_attribute *attr,
>
> static int __init pseries_dlpar_init(void)
> {
> + arch_update_cpu_topology();
> +
> pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue",
> WQ_UNBOUND, 1);
> return sysfs_create_file(kernel_kobj, &class_attr_dlpar.attr);
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 7bc0e91..b5eff35 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -619,7 +619,8 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
> }
>
> rc = dlpar_cpu_remove(dn, drc_index);
> - of_node_put(dn);
> + if (rc)
> + of_node_put(dn);
This if statement is unnecessary. A reference to dn is grabbed with
cpu_drc_index_to_dn()
earlier in the function. So, regardless of whether dlpar_cpu_remove() succeeds
or fails we
still need to release the reference we took. I suspect that maybe you threw
this in there
to prevent the of_node underflow that was being seen during dlpar remove of
cpus. A fix
for that should already be upstream. Either way this hunk doesn't seem to have
anything to
do with the rest of this VPHN patch, and should have been a separate patch with
its own
change log explaining the reasoning.
-Tyrel
> return rc;
> }
>
>
