On 09/01/2017 10:48 AM, Michael Bringmann wrote:
> powerpc/hotplug: 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. During hotplug
> CPU operations, this patch resets the timer on topology update work
> function to a small value to better ensure that the CPU topology is
> detected and configured sooner.
Looking through the changes you've made here I don't see where the
topology timeout ever gets set to the default timeout. When calculating
the next timeout you use topology_timer_secs which is initialized to 1, so
the timer pops every second after initialization. Then after a dlpar cpu
operation the timer is set to pop every second. There is no place that I
see where the timeout is set to the default 60 seconds.
>
> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/topology.h | 8 ++++++++
> arch/powerpc/mm/numa.c | 21 ++++++++++++++++++++-
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 ++
> 3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/topology.h
> b/arch/powerpc/include/asm/topology.h
> index dc4e159..beb9bca 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -98,6 +98,14 @@ static inline int prrn_is_enabled(void)
> }
> #endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
>
> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_NEED_MULTIPLE_NODES)
> +#if defined(CONFIG_PPC_SPLPAR)
> +extern int timed_topology_update(int nsecs);
> +#else
> +#define timed_topology_update(nsecs)
> +#endif /* CONFIG_PPC_SPLPAR */
> +#endif /* CONFIG_HOTPLUG_CPU || CONFIG_NEED_MULTIPLE_NODES */
> +
> #include <asm-generic/topology.h>
>
> #ifdef CONFIG_SMP
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index c08d736..3a5b334 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1148,15 +1148,34 @@ struct topology_update_data {
> int new_nid;
> };
>
> +#define TOPOLOGY_DEF_TIMER_SECS 60
> +
> static u8 vphn_cpu_change_counts[NR_CPUS][MAX_DISTANCE_REF_POINTS];
> static cpumask_t cpu_associativity_changes_mask;
> static int vphn_enabled;
> static int prrn_enabled;
> static void reset_topology_timer(void);
> +static int topology_timer_secs = 1;
> static int topology_inited;
> static int topology_update_needed;
>
> /*
> + * Change polling interval for associativity changes.
> + */
> +int timed_topology_update(int nsecs)
> +{
> + if (nsecs > 0)
> + topology_timer_secs = nsecs;
> + else
> + topology_timer_secs = TOPOLOGY_DEF_TIMER_SECS;
> +
> + if (vphn_enabled)
> + reset_topology_timer();
Should this whole thing be wrapped by if (vphn_enabled) ?
-Nathan
> +
> + return 0;
> +}
> +
> +/*
> * Store the current values of the associativity change counters in the
> * hypervisor.
> */
> @@ -1489,7 +1508,7 @@ static void topology_timer_fn(unsigned long ignored)
> static void reset_topology_timer(void)
> {
> topology_timer.data = 0;
> - topology_timer.expires = jiffies + 60 * HZ;
> + topology_timer.expires = jiffies + topology_timer_secs * HZ;
> mod_timer(&topology_timer, topology_timer.expires);
> }
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 6afd1ef..5a7fb1e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -356,6 +356,7 @@ static int dlpar_online_cpu(struct device_node *dn)
> BUG_ON(get_cpu_current_state(cpu)
> != CPU_STATE_OFFLINE);
> cpu_maps_update_done();
> + timed_topology_update(1);
> rc = device_online(get_cpu_device(cpu));
> if (rc)
> goto out;
> @@ -522,6 +523,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
> set_preferred_offline_state(cpu,
> CPU_STATE_OFFLINE);
> cpu_maps_update_done();
> + timed_topology_update(1);
> rc = device_offline(get_cpu_device(cpu));
> if (rc)
> goto out;
>
