From nobody Thu Dec 25 19:53:26 2025 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9BC2D56759; Thu, 11 Jan 2024 20:55:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="A+Junh4Q"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="jB2On499" Date: Thu, 11 Jan 2024 20:55:51 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1705006553; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ew8ynsOxol/v9waaBF0d1CRM/05/wdsVBj8lJ/MPY0Q=; b=A+Junh4Q3BcCHInz5PMKRBEjtnWHBjd6xRBALJkrRhej8Oh9t6VBK/jMb9QNvhQndPgr+R su6lnf+nOkSQjshbGMhgzX7X454HiSyROkeGb8fgNsxmxyCTSdyoqkf+a2i1MeEWEVrgZq E/PRAL16GZJ8TBy1FA01iU8GTtfe76V/aDd/lW1bWPKcSxYjqYEMMJol6Hn2lBvCXq1gcA /ybAor7Lt5fkzjX9BHAsbrFjCRpeV2G1sfFbVjDh4RXCeIxMB5anwo1XFvy52ckSiwKJaa IfgoG/ii4nkc5g7rqHmds7/RZV1aO/tYhF9767MF5CEYejveHlMIviwpT+Arqw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1705006553; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ew8ynsOxol/v9waaBF0d1CRM/05/wdsVBj8lJ/MPY0Q=; b=jB2On4992/JiFaRuG5RQErm0zfbUU8wG9g7X3AjAPBb/e1539vHJr4Le7j5XQV3zfJfQ56 93ywwGMKSRiqjZCg== From: "tip-bot2 for Ingo Molnar" Sender: tip-bot2@linutronix.de Reply-to: linux-kernel@vger.kernel.org To: linux-tip-commits@vger.kernel.org Subject: [tip: sched/urgent] Revert "sched/cpufreq: Rework schedutil governor performance estimation" and dependent commit Cc: Linus Torvalds , Ingo Molnar , Vincent Guittot , x86@kernel.org, linux-kernel@vger.kernel.org In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-ID: <170500655188.398.4921277065850419418.tip-bot2@tip-bot2> Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails Precedence: bulk Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable The following commit has been merged into the sched/urgent branch of tip: Commit-ID: 60ee1706bd11669b0530efb4e92526854b9a6364 Gitweb: https://git.kernel.org/tip/60ee1706bd11669b0530efb4e92526854= b9a6364 Author: Ingo Molnar AuthorDate: Thu, 11 Jan 2024 11:45:17 +01:00 Committer: Ingo Molnar CommitterDate: Thu, 11 Jan 2024 21:42:42 +01:00 Revert "sched/cpufreq: Rework schedutil governor performance estimation" an= d dependent commit This reverts the following commits: f12560779f9d73 ("sched/cpufreq: Rework iowait boost") 9c0b4bb7f6303c ("sched/cpufreq: Rework schedutil governor performance est= imation") Because Linus reported a bad performance regression with the sched_util governor, that increased the time his empty kernel build took from 22 to 44 seconds (and can be similarly measured in full builds as well) - and bisected it back to 9c0b4bb7f6303c. Until we have a proper fix, revert the broken commit and its dependent commit. Reported-by: Linus Torvalds Bisected-by: Linus Torvalds Signed-off-by: Ingo Molnar Cc: Vincent Guittot Link: https://lore.kernel.org/r/CAHk-=3DwgWcYX2oXKtgvNN2LLDXP7kXkbo-xTfumEj= mPbjSer2RQ@mail.gmail.com --- include/linux/energy_model.h | 1 +- kernel/sched/core.c | 90 +++++++++++++++++-------------- kernel/sched/cpufreq_schedutil.c | 64 ++++++++-------------- kernel/sched/fair.c | 22 +------- kernel/sched/sched.h | 84 ++++++++++++++++++++++++++--- 5 files changed, 158 insertions(+), 103 deletions(-) diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h index 88d91e0..c19e7ef 100644 --- a/include/linux/energy_model.h +++ b/include/linux/energy_model.h @@ -243,6 +243,7 @@ static inline unsigned long em_cpu_energy(struct em_per= f_domain *pd, scale_cpu =3D arch_scale_cpu_capacity(cpu); ref_freq =3D arch_scale_freq_ref(cpu); =20 + max_util =3D map_util_perf(max_util); max_util =3D min(max_util, allowed_cpu_cap); freq =3D map_util_freq(max_util, ref_freq, scale_cpu); =20 diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 9116bcc..038eeaf 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7468,13 +7468,18 @@ int sched_core_idle_cpu(int cpu) * required to meet deadlines. */ unsigned long effective_cpu_util(int cpu, unsigned long util_cfs, - unsigned long *min, - unsigned long *max) + enum cpu_util_type type, + struct task_struct *p) { - unsigned long util, irq, scale; + unsigned long dl_util, util, irq, max; struct rq *rq =3D cpu_rq(cpu); =20 - scale =3D arch_scale_cpu_capacity(cpu); + max =3D arch_scale_cpu_capacity(cpu); + + if (!uclamp_is_used() && + type =3D=3D FREQUENCY_UTIL && rt_rq_is_runnable(&rq->rt)) { + return max; + } =20 /* * Early check to see if IRQ/steal time saturates the CPU, can be @@ -7482,49 +7487,45 @@ unsigned long effective_cpu_util(int cpu, unsigned = long util_cfs, * update_irq_load_avg(). */ irq =3D cpu_util_irq(rq); - if (unlikely(irq >=3D scale)) { - if (min) - *min =3D scale; - if (max) - *max =3D scale; - return scale; - } - - if (min) { - /* - * The minimum utilization returns the highest level between: - * - the computed DL bandwidth needed with the IRQ pressure which - * steals time to the deadline task. - * - The minimum performance requirement for CFS and/or RT. - */ - *min =3D max(irq + cpu_bw_dl(rq), uclamp_rq_get(rq, UCLAMP_MIN)); - - /* - * When an RT task is runnable and uclamp is not used, we must - * ensure that the task will run at maximum compute capacity. - */ - if (!uclamp_is_used() && rt_rq_is_runnable(&rq->rt)) - *min =3D max(*min, scale); - } + if (unlikely(irq >=3D max)) + return max; =20 /* * Because the time spend on RT/DL tasks is visible as 'lost' time to * CFS tasks and we use the same metric to track the effective * utilization (PELT windows are synchronized) we can directly add them * to obtain the CPU's actual utilization. + * + * CFS and RT utilization can be boosted or capped, depending on + * utilization clamp constraints requested by currently RUNNABLE + * tasks. + * When there are no CFS RUNNABLE tasks, clamps are released and + * frequency will be gracefully reduced with the utilization decay. */ util =3D util_cfs + cpu_util_rt(rq); - util +=3D cpu_util_dl(rq); + if (type =3D=3D FREQUENCY_UTIL) + util =3D uclamp_rq_util_with(rq, util, p); + + dl_util =3D cpu_util_dl(rq); =20 /* - * The maximum hint is a soft bandwidth requirement, which can be lower - * than the actual utilization because of uclamp_max requirements. + * For frequency selection we do not make cpu_util_dl() a permanent part + * of this sum because we want to use cpu_bw_dl() later on, but we need + * to check if the CFS+RT+DL sum is saturated (ie. no idle time) such + * that we select f_max when there is no idle time. + * + * NOTE: numerical errors or stop class might cause us to not quite hit + * saturation when we should -- something for later. */ - if (max) - *max =3D min(scale, uclamp_rq_get(rq, UCLAMP_MAX)); + if (util + dl_util >=3D max) + return max; =20 - if (util >=3D scale) - return scale; + /* + * OTOH, for energy computation we need the estimated running time, so + * include util_dl and ignore dl_bw. + */ + if (type =3D=3D ENERGY_UTIL) + util +=3D dl_util; =20 /* * There is still idle time; further improve the number by using the @@ -7535,15 +7536,28 @@ unsigned long effective_cpu_util(int cpu, unsigned = long util_cfs, * U' =3D irq + --------- * U * max */ - util =3D scale_irq_capacity(util, irq, scale); + util =3D scale_irq_capacity(util, irq, max); util +=3D irq; =20 - return min(scale, util); + /* + * Bandwidth required by DEADLINE must always be granted while, for + * FAIR and RT, we use blocked utilization of IDLE CPUs as a mechanism + * to gracefully reduce the frequency when no tasks show up for longer + * periods of time. + * + * Ideally we would like to set bw_dl as min/guaranteed freq and util + + * bw_dl as requested freq. However, cpufreq is not yet ready for such + * an interface. So, we only do the latter for now. + */ + if (type =3D=3D FREQUENCY_UTIL) + util +=3D cpu_bw_dl(rq); + + return min(max, util); } =20 unsigned long sched_cpu_util(int cpu) { - return effective_cpu_util(cpu, cpu_util_cfs(cpu), NULL, NULL); + return effective_cpu_util(cpu, cpu_util_cfs(cpu), ENERGY_UTIL, NULL); } #endif /* CONFIG_SMP */ =20 diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedu= til.c index 95c3c09..5f8729c 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -47,7 +47,7 @@ struct sugov_cpu { u64 last_update; =20 unsigned long util; - unsigned long bw_min; + unsigned long bw_dl; =20 /* The field below is for single-CPU policies only: */ #ifdef CONFIG_NO_HZ_COMMON @@ -164,6 +164,7 @@ static unsigned int get_next_freq(struct sugov_policy *= sg_policy, struct cpufreq_policy *policy =3D sg_policy->policy; unsigned int freq; =20 + util =3D map_util_perf(util); freq =3D get_capacity_ref_freq(policy); freq =3D map_util_freq(util, freq, max); =20 @@ -174,31 +175,14 @@ static unsigned int get_next_freq(struct sugov_policy= *sg_policy, return cpufreq_driver_resolve_freq(policy, freq); } =20 -unsigned long sugov_effective_cpu_perf(int cpu, unsigned long actual, - unsigned long min, - unsigned long max) +static void sugov_get_util(struct sugov_cpu *sg_cpu) { - /* Add dvfs headroom to actual utilization */ - actual =3D map_util_perf(actual); - /* Actually we don't need to target the max performance */ - if (actual < max) - max =3D actual; + unsigned long util =3D cpu_util_cfs_boost(sg_cpu->cpu); + struct rq *rq =3D cpu_rq(sg_cpu->cpu); =20 - /* - * Ensure at least minimum performance while providing more compute - * capacity when possible. - */ - return max(min, max); -} - -static void sugov_get_util(struct sugov_cpu *sg_cpu, unsigned long boost) -{ - unsigned long min, max, util =3D cpu_util_cfs_boost(sg_cpu->cpu); - - util =3D effective_cpu_util(sg_cpu->cpu, util, &min, &max); - util =3D max(util, boost); - sg_cpu->bw_min =3D min; - sg_cpu->util =3D sugov_effective_cpu_perf(sg_cpu->cpu, util, min, max); + sg_cpu->bw_dl =3D cpu_bw_dl(rq); + sg_cpu->util =3D effective_cpu_util(sg_cpu->cpu, util, + FREQUENCY_UTIL, NULL); } =20 /** @@ -289,16 +273,18 @@ static void sugov_iowait_boost(struct sugov_cpu *sg_c= pu, u64 time, * This mechanism is designed to boost high frequently IO waiting tasks, w= hile * being more conservative on tasks which does sporadic IO operations. */ -static unsigned long sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time, +static void sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time, unsigned long max_cap) { + unsigned long boost; + /* No boost currently required */ if (!sg_cpu->iowait_boost) - return 0; + return; =20 /* Reset boost if the CPU appears to have been idle enough */ if (sugov_iowait_reset(sg_cpu, time, false)) - return 0; + return; =20 if (!sg_cpu->iowait_boost_pending) { /* @@ -307,7 +293,7 @@ static unsigned long sugov_iowait_apply(struct sugov_cp= u *sg_cpu, u64 time, sg_cpu->iowait_boost >>=3D 1; if (sg_cpu->iowait_boost < IOWAIT_BOOST_MIN) { sg_cpu->iowait_boost =3D 0; - return 0; + return; } } =20 @@ -317,7 +303,10 @@ static unsigned long sugov_iowait_apply(struct sugov_c= pu *sg_cpu, u64 time, * sg_cpu->util is already in capacity scale; convert iowait_boost * into the same scale so we can compare. */ - return (sg_cpu->iowait_boost * max_cap) >> SCHED_CAPACITY_SHIFT; + boost =3D (sg_cpu->iowait_boost * max_cap) >> SCHED_CAPACITY_SHIFT; + boost =3D uclamp_rq_util_with(cpu_rq(sg_cpu->cpu), boost, NULL); + if (sg_cpu->util < boost) + sg_cpu->util =3D boost; } =20 #ifdef CONFIG_NO_HZ_COMMON @@ -339,7 +328,7 @@ static inline bool sugov_cpu_is_busy(struct sugov_cpu *= sg_cpu) { return false; } */ static inline void ignore_dl_rate_limit(struct sugov_cpu *sg_cpu) { - if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_min) + if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_dl) sg_cpu->sg_policy->limits_changed =3D true; } =20 @@ -347,8 +336,6 @@ static inline bool sugov_update_single_common(struct su= gov_cpu *sg_cpu, u64 time, unsigned long max_cap, unsigned int flags) { - unsigned long boost; - sugov_iowait_boost(sg_cpu, time, flags); sg_cpu->last_update =3D time; =20 @@ -357,8 +344,8 @@ static inline bool sugov_update_single_common(struct su= gov_cpu *sg_cpu, if (!sugov_should_update_freq(sg_cpu->sg_policy, time)) return false; =20 - boost =3D sugov_iowait_apply(sg_cpu, time, max_cap); - sugov_get_util(sg_cpu, boost); + sugov_get_util(sg_cpu); + sugov_iowait_apply(sg_cpu, time, max_cap); =20 return true; } @@ -442,8 +429,8 @@ static void sugov_update_single_perf(struct update_util= _data *hook, u64 time, sugov_cpu_is_busy(sg_cpu) && sg_cpu->util < prev_util) sg_cpu->util =3D prev_util; =20 - cpufreq_driver_adjust_perf(sg_cpu->cpu, sg_cpu->bw_min, - sg_cpu->util, max_cap); + cpufreq_driver_adjust_perf(sg_cpu->cpu, map_util_perf(sg_cpu->bw_dl), + map_util_perf(sg_cpu->util), max_cap); =20 sg_cpu->sg_policy->last_freq_update_time =3D time; } @@ -459,10 +446,9 @@ static unsigned int sugov_next_freq_shared(struct sugo= v_cpu *sg_cpu, u64 time) =20 for_each_cpu(j, policy->cpus) { struct sugov_cpu *j_sg_cpu =3D &per_cpu(sugov_cpu, j); - unsigned long boost; =20 - boost =3D sugov_iowait_apply(j_sg_cpu, time, max_cap); - sugov_get_util(j_sg_cpu, boost); + sugov_get_util(j_sg_cpu); + sugov_iowait_apply(j_sg_cpu, time, max_cap); =20 util =3D max(j_sg_cpu->util, util); } diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 533547e..f2bb836 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -7820,7 +7820,7 @@ static inline void eenv_pd_busy_time(struct energy_en= v *eenv, for_each_cpu(cpu, pd_cpus) { unsigned long util =3D cpu_util(cpu, p, -1, 0); =20 - busy_time +=3D effective_cpu_util(cpu, util, NULL, NULL); + busy_time +=3D effective_cpu_util(cpu, util, ENERGY_UTIL, NULL); } =20 eenv->pd_busy_time =3D min(eenv->pd_cap, busy_time); @@ -7843,7 +7843,7 @@ eenv_pd_max_util(struct energy_env *eenv, struct cpum= ask *pd_cpus, for_each_cpu(cpu, pd_cpus) { struct task_struct *tsk =3D (cpu =3D=3D dst_cpu) ? p : NULL; unsigned long util =3D cpu_util(cpu, p, dst_cpu, 1); - unsigned long eff_util, min, max; + unsigned long eff_util; =20 /* * Performance domain frequency: utilization clamping @@ -7852,23 +7852,7 @@ eenv_pd_max_util(struct energy_env *eenv, struct cpu= mask *pd_cpus, * NOTE: in case RT tasks are running, by default the * FREQUENCY_UTIL's utilization can be max OPP. */ - eff_util =3D effective_cpu_util(cpu, util, &min, &max); - - /* Task's uclamp can modify min and max value */ - if (tsk && uclamp_is_used()) { - min =3D max(min, uclamp_eff_value(p, UCLAMP_MIN)); - - /* - * If there is no active max uclamp constraint, - * directly use task's one, otherwise keep max. - */ - if (uclamp_rq_is_idle(cpu_rq(cpu))) - max =3D uclamp_eff_value(p, UCLAMP_MAX); - else - max =3D max(max, uclamp_eff_value(p, UCLAMP_MAX)); - } - - eff_util =3D sugov_effective_cpu_perf(cpu, eff_util, min, max); + eff_util =3D effective_cpu_util(cpu, util, FREQUENCY_UTIL, tsk); max_util =3D max(max_util, eff_util); } =20 diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 001fe04..eb7e07a 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -3005,14 +3005,24 @@ static inline void cpufreq_update_util(struct rq *r= q, unsigned int flags) {} #endif =20 #ifdef CONFIG_SMP -unsigned long effective_cpu_util(int cpu, unsigned long util_cfs, - unsigned long *min, - unsigned long *max); - -unsigned long sugov_effective_cpu_perf(int cpu, unsigned long actual, - unsigned long min, - unsigned long max); +/** + * enum cpu_util_type - CPU utilization type + * @FREQUENCY_UTIL: Utilization used to select frequency + * @ENERGY_UTIL: Utilization used during energy calculation + * + * The utilization signals of all scheduling classes (CFS/RT/DL) and IRQ t= ime + * need to be aggregated differently depending on the usage made of them. = This + * enum is used within effective_cpu_util() to differentiate the types of + * utilization expected by the callers, and adjust the aggregation accordi= ngly. + */ +enum cpu_util_type { + FREQUENCY_UTIL, + ENERGY_UTIL, +}; =20 +unsigned long effective_cpu_util(int cpu, unsigned long util_cfs, + enum cpu_util_type type, + struct task_struct *p); =20 /* * Verify the fitness of task @p to run on @cpu taking into account the @@ -3069,6 +3079,59 @@ static inline bool uclamp_rq_is_idle(struct rq *rq) return rq->uclamp_flags & UCLAMP_FLAG_IDLE; } =20 +/** + * uclamp_rq_util_with - clamp @util with @rq and @p effective uclamp valu= es. + * @rq: The rq to clamp against. Must not be NULL. + * @util: The util value to clamp. + * @p: The task to clamp against. Can be NULL if you want to clamp + * against @rq only. + * + * Clamps the passed @util to the max(@rq, @p) effective uclamp values. + * + * If sched_uclamp_used static key is disabled, then just return the util + * without any clamping since uclamp aggregation at the rq level in the fa= st + * path is disabled, rendering this operation a NOP. + * + * Use uclamp_eff_value() if you don't care about uclamp values at rq leve= l. It + * will return the correct effective uclamp value of the task even if the + * static key is disabled. + */ +static __always_inline +unsigned long uclamp_rq_util_with(struct rq *rq, unsigned long util, + struct task_struct *p) +{ + unsigned long min_util =3D 0; + unsigned long max_util =3D 0; + + if (!static_branch_likely(&sched_uclamp_used)) + return util; + + if (p) { + min_util =3D uclamp_eff_value(p, UCLAMP_MIN); + max_util =3D uclamp_eff_value(p, UCLAMP_MAX); + + /* + * Ignore last runnable task's max clamp, as this task will + * reset it. Similarly, no need to read the rq's min clamp. + */ + if (uclamp_rq_is_idle(rq)) + goto out; + } + + min_util =3D max_t(unsigned long, min_util, uclamp_rq_get(rq, UCLAMP_MIN)= ); + max_util =3D max_t(unsigned long, max_util, uclamp_rq_get(rq, UCLAMP_MAX)= ); +out: + /* + * Since CPU's {min,max}_util clamps are MAX aggregated considering + * RUNNABLE tasks with _different_ clamps, we can end up with an + * inversion. Fix it now when the clamps are applied. + */ + if (unlikely(min_util >=3D max_util)) + return min_util; + + return clamp(util, min_util, max_util); +} + /* Is the rq being capped/throttled by uclamp_max? */ static inline bool uclamp_rq_is_capped(struct rq *rq) { @@ -3106,6 +3169,13 @@ static inline unsigned long uclamp_eff_value(struct = task_struct *p, return SCHED_CAPACITY_SCALE; } =20 +static inline +unsigned long uclamp_rq_util_with(struct rq *rq, unsigned long util, + struct task_struct *p) +{ + return util; +} + static inline bool uclamp_rq_is_capped(struct rq *rq) { return false; } =20 static inline bool uclamp_is_used(void)