Received-SPF: pass (google.com: domain of linux-kernel+bounces-94615-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 6 Mar 2024 21:47:04 +0000
From: Qais Yousef <qyousef@layalina.io>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ingo Molnar <mingo@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	linux-kernel@vger.kernel.org,
	Pierre Gondois <Pierre.Gondois@arm.com>
Subject: Re: [PATCH v6 2/4] sched/fair: Check a task has a fitting cpu when
 updating misfit
Message-ID: <20240306214704.uditboboedut2lm2@airbuntu>
References: <20240220225622.2626569-1-qyousef@layalina.io>
 <20240220225622.2626569-3-qyousef@layalina.io>
 <d6699c3a-3df6-46a3-98db-e07c8722f106@arm.com>
 <20240303174416.7m3gv5wywcmedov4@airbuntu>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20240303174416.7m3gv5wywcmedov4@airbuntu>

On 03/03/24 17:44, Qais Yousef wrote:

> 	diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> 	index 174687252e1a..b0e60a565945 100644
> 	--- a/kernel/sched/fair.c
> 	+++ b/kernel/sched/fair.c
> 	@@ -8260,6 +8260,8 @@ static void set_task_max_allowed_capacity(struct task_struct *p)
> 			cpumask_t *cpumask;
> 	 
> 			cpumask = cpu_capacity_span(entry);
> 	+		if (!cpumask_intersects(cpu_active_mask, cpumask))
> 	+			continue;
> 			if (!cpumask_intersects(p->cpus_ptr, cpumask))
> 				continue;
> 	 
> 	@@ -8269,6 +8271,53 @@ static void set_task_max_allowed_capacity(struct task_struct *p)
> 		rcu_read_unlock();
> 	 }
> 	 
> 	+static void __update_tasks_max_allowed_capacity(unsigned long capacity)
> 	+{
> 	+	struct task_struct *g, *p;
> 	+
> 	+	for_each_process_thread(g, p) {
> 	+		if (fair_policy(p->policy) && p->max_allowed_capacity == capacity)

This condition actually not good enough. We need to differentiate between going
online/offline. I didn't want to call set_task_max_allowed_capacity()
unconditionally and make hotplug even slower.

I'm doing more testing and will post v8 once done. I need to cater for a new
user when dynamic EM changes capacities too.. Small things can snow ball easily
hehe.

> 	+			set_task_max_allowed_capacity(p);
> 	+	}
> 	+}
> 	+
> 	+/*
> 	+ * Handle a cpu going online/offline changing the available capacity levels.
> 	+ */
> 	+static void update_tasks_max_allowed_capacity(int cpu, bool online)
> 	+{
> 	+	struct asym_cap_data *entry;
> 	+	bool do_update = false;
> 	+
> 	+	if (!sched_asym_cpucap_active())
> 	+		return;
> 	+
> 	+	if (cpuhp_tasks_frozen)
> 	+		return;
> 	+
> 	+	rcu_read_lock();
> 	+	/* Did a capacity level appear/disappear? */
> 	+	list_for_each_entry_rcu(entry, &asym_cap_list, link) {
> 	+		unsigned int nr_active;
> 	+		cpumask_t *cpumask;
> 	+
> 	+		cpumask = cpu_capacity_span(entry);
> 	+
> 	+		if (!cpumask_test_cpu(cpu, cpumask))
> 	+			continue;
> 	+
> 	+		nr_active = cpumask_weight_and(cpu_active_mask, cpumask);
> 	+		if (online)
> 	+			do_update = nr_active == 1;
> 	+		else
> 	+			do_update = !nr_active;
> 	+		break;
> 	+	}
> 	+	if (do_update)
> 	+		__update_tasks_max_allowed_capacity(entry->capacity);
> 	+	rcu_read_unlock();
> 	+}
> 	+
> 	 static void set_cpus_allowed_fair(struct task_struct *p, struct affinity_context *ctx)
> 	 {
> 		set_cpus_allowed_common(p, ctx);
> 	@@ -12500,6 +12549,8 @@ static void rq_online_fair(struct rq *rq)
> 		update_sysctl();
> 	 
> 		update_runtime_enabled(rq);
> 	+
> 	+	update_tasks_max_allowed_capacity(cpu_of(rq), true);
> 	 }
> 	 
> 	 static void rq_offline_fair(struct rq *rq)
> 	@@ -12511,6 +12562,8 @@ static void rq_offline_fair(struct rq *rq)
> 	 
> 		/* Ensure that we remove rq contribution to group share: */
> 		clear_tg_offline_cfs_rqs(rq);
> 	+
> 	+	update_tasks_max_allowed_capacity(cpu_of(rq), false);
> 	 }
> 	 
> 	 #endif /* CONFIG_SMP */
> -- 
> 2.34.1