[PATCH 0/2] timers/migration: Fixes on top of per capacity hierarchies

[PATCH 0/2] timers/migration: Fixes on top of per capacity hierarchies

[Frederic Weisbecker]

Hi,

Looks like I overlooked a few things when I worked on making timer
migration capacity aware. Those patches apply on top of tip:timers/core

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	timers/fixes

HEAD: 2b343fb3e17b72d6c26c9cff29fe8e53493b4cb9

Thanks,
	Frederic
---

Frederic Weisbecker (2):
      timers/migration: Fix hotplug migrator selection target on asymetric capacity machines
      timers/migration: Deactivate per-capacity hierarchies under nohz_full

 kernel/time/timer_migration.c | 64 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 13 deletions(-)

[PATCH 1/2] timers/migration: Fix hotplug migrator selection target on asymetric capacity machines

[Frederic Weisbecker]

When a top-level migrator is deactivated, either at CPU down hotplug
time or when a CPU is domain isolated, a new migrator is elected among
the available CPUs and woken up to take over the migration duty.

However that election must happen at the scope of a given hierarchy and
not globally, which the introduction of per-capacity hierarchies failed
to handle.

As a result a given hierarchy may end up without migrator to handle
global timers.

Fix it with making sure that the new migrator belongs to the same
hierarchy as the outgoing CPU.

Fixes: 098cbaad8e57 ("timers/migration: Split per-capacity hierarchies")
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/timer_migration.c | 42 ++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 25e3c563eb74..8032b0044f44 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -1464,6 +1464,18 @@ static long tmigr_trigger_active(void *unused)
 	return 0;
 }
 
+static struct tmigr_hierarchy *__tmigr_get_hierarchy(unsigned int capacity)
+{
+	struct tmigr_hierarchy *iter;
+
+	list_for_each_entry(iter, &tmigr_hierarchy_list, node) {
+		if (iter->capacity == capacity)
+			return iter;
+	}
+
+	return NULL;
+}
+
 static int tmigr_clear_cpu_available(unsigned int cpu)
 {
 	struct tmigr_cpu *tmc = this_cpu_ptr(&tmigr_cpu);
@@ -1488,8 +1500,21 @@ static int tmigr_clear_cpu_available(unsigned int cpu)
 	}
 
 	if (firstexp != KTIME_MAX) {
-		migrator = cpumask_any(tmigr_available_cpumask);
-		work_on_cpu(migrator, tmigr_trigger_active, NULL);
+		struct tmigr_hierarchy *hier = __tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
+
+		if (WARN_ON_ONCE(!hier))
+			return -EINVAL;
+
+		migrator = cpumask_any_and(tmigr_available_cpumask, hier->cpumask);
+		if (migrator < nr_cpu_ids) {
+			work_on_cpu(migrator, tmigr_trigger_active, NULL);
+		} else {
+			/*
+			 * If deactivation returned an expiration, it belongs to an available
+			 * nohz CPU in the hierarchy.
+			 */
+			WARN_ONCE(1, "Expected available CPU in the hierarchy\n");
+		}
 	}
 
 	return 0;
@@ -1915,12 +1940,9 @@ static int tmigr_setup_groups(struct tmigr_hierarchy *hier, unsigned int cpu,
 
 static struct tmigr_hierarchy *tmigr_get_hierarchy(unsigned int capacity)
 {
-	struct tmigr_hierarchy *hier = NULL, *iter;
+	struct tmigr_hierarchy *hier;
 
-	list_for_each_entry(iter, &tmigr_hierarchy_list, node) {
-		if (iter->capacity == capacity)
-			hier = iter;
-	}
+	hier = __tmigr_get_hierarchy(capacity);
 
 	if (hier)
 		return hier;
@@ -1978,9 +2000,9 @@ static long connect_old_root_work(void *arg)
 	struct tmigr_hierarchy *hier;
 	int cpu = smp_processor_id();
 
-	hier = tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
-	if (IS_ERR(hier))
-		return PTR_ERR(hier);
+	hier = __tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
+	if (WARN_ON_ONCE(!hier))
+		return -EINVAL;
 
 	return tmigr_connect_old_root(hier, cpu, old_root, true);
 }
-- 
2.53.0

[PATCH 2/2] timers/migration: Deactivate per-capacity hierarchies under nohz_full

[Frederic Weisbecker]

Nohz_full CPUs global timers are guaranteed to be handled by the
timekeeper: that CPU never stops its tick and therefore remains active
in the hierarchy.

But since the introduction of per-capacity hierarchies, this guarantee
is broken because the timekeeper may not belong to the same hierarchy
as all the nohz_full CPUs.

Fix it with simply turning off capacity awareness when nohz_full is
running and force a single hierarchy. Nohz_full is not exactly optimized
powerwise anyway.

Fixes: 098cbaad8e57 ("timers/migration: Split per-capacity hierarchies")
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/time/timer_migration.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 8032b0044f44..8ba53ad49173 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -1464,8 +1464,24 @@ static long tmigr_trigger_active(void *unused)
 	return 0;
 }
 
-static struct tmigr_hierarchy *__tmigr_get_hierarchy(unsigned int capacity)
+static unsigned int tmigr_get_capacity(int cpu)
 {
+	/*
+	 * nohz_full CPUs need to make sure there is always an available (online)
+	 * and never idle migrator to handle all their global timers. That duty
+	 * is served by the timekeeper which then never stops its tick. But the
+	 * timekeeper must then belong to the same hierarchy as all the nohz_full
+	 * CPUs. Simply turn off capacity awareness when nohz_full is running.
+	 */
+	if (tick_nohz_full_enabled())
+		return SCHED_CAPACITY_SCALE;
+	else
+		return arch_scale_cpu_capacity(cpu);
+}
+
+static struct tmigr_hierarchy *__tmigr_get_hierarchy(int cpu)
+{
+	unsigned int capacity = tmigr_get_capacity(cpu);
 	struct tmigr_hierarchy *iter;
 
 	list_for_each_entry(iter, &tmigr_hierarchy_list, node) {
@@ -1500,7 +1516,7 @@ static int tmigr_clear_cpu_available(unsigned int cpu)
 	}
 
 	if (firstexp != KTIME_MAX) {
-		struct tmigr_hierarchy *hier = __tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
+		struct tmigr_hierarchy *hier = __tmigr_get_hierarchy(cpu);
 
 		if (WARN_ON_ONCE(!hier))
 			return -EINVAL;
@@ -1938,11 +1954,11 @@ static int tmigr_setup_groups(struct tmigr_hierarchy *hier, unsigned int cpu,
 	return err;
 }
 
-static struct tmigr_hierarchy *tmigr_get_hierarchy(unsigned int capacity)
+static struct tmigr_hierarchy *tmigr_get_hierarchy(int cpu)
 {
 	struct tmigr_hierarchy *hier;
 
-	hier = __tmigr_get_hierarchy(capacity);
+	hier = __tmigr_get_hierarchy(cpu);
 
 	if (hier)
 		return hier;
@@ -1962,7 +1978,7 @@ static struct tmigr_hierarchy *tmigr_get_hierarchy(unsigned int capacity)
 	for (int i = 0; i < tmigr_hierarchy_levels; i++)
 		INIT_LIST_HEAD(&hier->level_list[i]);
 
-	hier->capacity = capacity;
+	hier->capacity = tmigr_get_capacity(cpu);
 	list_add_tail(&hier->node, &tmigr_hierarchy_list);
 
 	return hier;
@@ -2000,7 +2016,7 @@ static long connect_old_root_work(void *arg)
 	struct tmigr_hierarchy *hier;
 	int cpu = smp_processor_id();
 
-	hier = __tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
+	hier = __tmigr_get_hierarchy(cpu);
 	if (WARN_ON_ONCE(!hier))
 		return -EINVAL;
 
@@ -2016,7 +2032,7 @@ static int tmigr_add_cpu(unsigned int cpu)
 
 	guard(mutex)(&tmigr_mutex);
 
-	hier = tmigr_get_hierarchy(arch_scale_cpu_capacity(cpu));
+	hier = tmigr_get_hierarchy(cpu);
 	if (IS_ERR(hier))
 		return PTR_ERR(hier);
 
-- 
2.53.0