From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 5443024A7C6 for ; Thu, 16 Jan 2025 17:34:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737048889; cv=none; b=BY1JFpnPAHultglmINVw9POYIL+9ZKbi7PnUHjg9t2KaQpzYcfhQqox6yveufZhxP5HEXjELMXDAKijF94iu1RlUitaUQ/0aHz930HINupO0sh766rK/51twdQzCOWfkNe877h8H+6LT6jnf5CVV+g56UdRqyrtc1RKePM93UgU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737048889; c=relaxed/simple; bh=WNq/zUS/cxWziESN2+zrYorG99cFrXsYBzy55YgvOtA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=G+Fv/VPcnHWdJIxccQmQFn7K4Amw6OT7/TpwHryZEPGo2LNfAX9XjjdeNJ3CPemtZxlGIcIz9Cqf6Vsw4MLG8VpsS2J5dh751LKhWQchAsv6mHs4kXV1T2Sv/4rDSALr8SqZfmrjdX2nihtBXizQeaYzwa1tmXwnfSJvUnhZpIA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 926D9106F; Thu, 16 Jan 2025 09:35:13 -0800 (PST) Received: from [10.154.93.158] (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3DFBD3F73F; Thu, 16 Jan 2025 09:34:38 -0800 (PST) Message-ID: <0c3312f8-4e1c-49df-a084-8c236c9f7f53@arm.com> Date: Thu, 16 Jan 2025 18:34:30 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 5/7 v2] sched/fair: Add push task callback for EAS To: Vincent Guittot , mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, lukasz.luba@arm.com, rafael.j.wysocki@intel.com, linux-kernel@vger.kernel.org Cc: qyousef@layalina.io, hongyan.xia2@arm.com, christian.loehle@arm.com, qperret@google.com References: <20241217160720.2397239-1-vincent.guittot@linaro.org> <20241217160720.2397239-6-vincent.guittot@linaro.org> Content-Language: en-US From: Pierre Gondois In-Reply-To: <20241217160720.2397239-6-vincent.guittot@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Vincent, On 12/17/24 17:07, Vincent Guittot wrote: > EAS is based on wakeup events to efficiently place tasks on the system, but > there are cases where a task will not have wakeup events anymore or at a > far too low pace. For such situation, we can take advantage of the task > being put back in the enqueued list to check if it should be migrated on > another CPU. > > Wake up events remain the main way to migrate tasks but we now detect > situation where a task is stuck on a CPU by checking that its utilization > is larger than the max available compute capacity (max cpu capacity or > uclamp max setting) It seems there are 2 distinct cases: a- The task is alone on a rq b- The task shares the rq and is enqueued/dequeued a. doesn't seem to need any of the push functions, and b. doesn't seem to need any of the misfit functions. Maybe it's worth splitting the patch in 2. > > Signed-off-by: Vincent Guittot > --- > kernel/sched/fair.c | 206 +++++++++++++++++++++++++++++++++++++++++++ > kernel/sched/sched.h | 2 + > 2 files changed, 208 insertions(+) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index cd046e8216a9..2affc063da55 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -7088,6 +7088,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags) > hrtick_update(rq); > } > > +static void dequeue_pushable_task(struct rq *rq, struct task_struct *p); > static void set_next_buddy(struct sched_entity *se); > > /* > @@ -7118,6 +7119,9 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags) > h_nr_idle = task_has_idle_policy(p); > if (task_sleep || task_delayed || !se->sched_delayed) > h_nr_runnable = 1; > + > + if (task_sleep || task_on_rq_migrating(p)) > + dequeue_pushable_task(rq, p); > } else { > cfs_rq = group_cfs_rq(se); > slice = cfs_rq_min_slice(cfs_rq); > @@ -8617,6 +8621,182 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu) > return target; > } > > +static inline bool task_misfit_cpu(struct task_struct *p, int cpu) > +{ > + unsigned long max_capa = get_actual_cpu_capacity(cpu); > + unsigned long util = task_util_est(p); > + > + max_capa = min(max_capa, uclamp_eff_value(p, UCLAMP_MAX)); > + util = max(util, task_runnable(p)); > + > + /* > + * Return true only if the task might not sleep/wakeup because of a low > + * compute capacity. Tasks, which wake up regularly, will be handled by > + * feec(). > + */ NIT: On a little CPU with min_OPP=256 and max_OPP=512, a task with a util=100 and U_Max=10 will trigger this condition. However: - the task is already well placed from a power PoV - the tasks has opportunities to sleep/wake-up Shouldn't we ideally take: unsigned long max_capa; max_capa = max(min_capa(cpu), uclamp_eff_value(p, UCLAMP_MAX)); max_capa = min(get_actual_cpu_capacity(cpu), max_capa); with min_capa(cpu) returning 256 in this case, i.e. the CPU capacity at the lowest OPP ? > + return (util > max_capa); > +} > + [...]