On 09/01/2017 10:48 AM, Michael Bringmann wrote:
> powerpc/vphn: On Power systems with shared configurations of CPUs
> and memory, there are some issues with the association of additional
> CPUs and memory to nodes when hot-adding resources. This patch
> corrects the currently broken capability to set the topology for
> shared CPUs in LPARs. At boot time for shared CPU lpars, the
> topology for each CPU was being set to node zero. Now when
> numa_update_cpu_topology() is called appropriately, the Virtual
> Processor Home Node (VPHN) capabilities information provided by the
> pHyp allows the appropriate node in the shared configuration to be
> selected for the CPU.
>
> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
> ---
> Changes in V13:
> -- Split patch for improved review
> ---
> arch/powerpc/mm/numa.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index b95c584..312f6ee 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1153,6 +1153,8 @@ struct topology_update_data {
> static int vphn_enabled;
> static int prrn_enabled;
> static void reset_topology_timer(void);
> +static int topology_inited;
> +static int topology_update_needed;
>
> /*
> * Store the current values of the associativity change counters in the
> @@ -1246,6 +1248,11 @@ static long vphn_get_associativity(unsigned long cpu,
> "hcall_vphn() experienced a hardware fault "
> "preventing VPHN. Disabling polling...\n");
> stop_topology_update();
> + break;
> + case H_SUCCESS:
> + dbg("VPHN hcall succeeded. Reset polling...\n");
> + timed_topology_update(0);
> + break;
> }
>
> return rc;
> @@ -1323,8 +1330,11 @@ int numa_update_cpu_topology(bool cpus_locked)
> 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)
> @@ -1363,6 +1373,8 @@ int numa_update_cpu_topology(bool cpus_locked)
> 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);
As mentioned previously, this should either be removed or changed to a
debug statement.
> cpu = cpu_last_thread_sibling(cpu);
> continue;
> }
> @@ -1433,6 +1445,7 @@ int numa_update_cpu_topology(bool cpus_locked)
>
> out:
> kfree(updates);
> + topology_update_needed = 0;
> return changed;
> }
>
> @@ -1453,6 +1466,13 @@ static void topology_schedule_update(void)
> schedule_work(&topology_work);
> }
>
> +void shared_topology_update(void)
> +{
> + if (firmware_has_feature(FW_FEATURE_VPHN) &&
> + lppaca_shared_proc(get_lppaca()))
Could we just check vphn_enabled here? The init routine will set this to true
only if the feature is supported and we are using shared processors.
Also, this routine seems a bit odd. The only place it is called from is an
init routine, topology_update_init. Is there a reason this check couldn't just
be in that routine, or it seems like you could just call
topology_schedule_update
directly from start_topology_update when vphn is initialized.
-Nathan
> + topology_schedule_update();
> +}
> +
> static void topology_timer_fn(unsigned long ignored)
> {
> if (prrn_enabled && cpumask_weight(&cpu_associativity_changes_mask))
> @@ -1519,7 +1539,6 @@ int start_topology_update(void)
> if (firmware_has_feature(FW_FEATURE_PRRN)) {
> if (!prrn_enabled) {
> prrn_enabled = 1;
> - vphn_enabled = 0;
> #ifdef CONFIG_SMP
> rc = of_reconfig_notifier_register(&dt_update_nb);
> #endif
> @@ -1527,7 +1546,6 @@ int start_topology_update(void)
> } else if (firmware_has_feature(FW_FEATURE_VPHN) &&
> lppaca_shared_proc(get_lppaca())) {
> if (!vphn_enabled) {
> - prrn_enabled = 0;
> vphn_enabled = 1;
> setup_cpu_associativity_change_counters();
> init_timer_deferrable(&topology_timer);
> @@ -1613,9 +1631,16 @@ static int topology_update_init(void)
> if (topology_updates_enabled)
> start_topology_update();
>
> + shared_topology_update();
> +
> 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);
>
