linux-kernel - Re: [RFC PATCH 14/16] sched/schedutil: Ignore dvfs headroom when util is decaying

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZsbM1IiZyAuv7xz_@sultan-box.localdomain>
Date: Wed, 21 Aug 2024 22:29:56 -0700
From: "Sultan Alsawaf (unemployed)" <sultan@...neltoast.com>
To: Qais Yousef <qyousef@...alina.io>
Cc: Ingo Molnar <mingo@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	John Stultz <jstultz@...gle.com>, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 14/16] sched/schedutil: Ignore dvfs headroom when
 util is decaying

On Tue, Aug 20, 2024 at 05:35:10PM +0100, Qais Yousef wrote:
> It means we're being idling or doing less work and are already running
> at a higher value. No need to apply any dvfs headroom in this case.
> 
> Signed-off-by: Qais Yousef <qyousef@...alina.io>
> ---
>  kernel/sched/cpufreq_schedutil.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 318b09bc4ab1..4a1a8b353d51 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -9,6 +9,7 @@
>  #define IOWAIT_BOOST_MIN	(SCHED_CAPACITY_SCALE / 8)
>  
>  DEFINE_PER_CPU_READ_MOSTLY(unsigned long, response_time_mult);
> +DEFINE_PER_CPU(unsigned long, last_update_util);
>  
>  struct sugov_tunables {
>  	struct gov_attr_set	attr_set;
> @@ -262,15 +263,19 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
>   * Also take into accounting how long tasks have been waiting in runnable but
>   * !running state. If it is high, it means we need higher DVFS headroom to
>   * reduce it.
> - *
> - * XXX: Should we provide headroom when the util is decaying?
>   */
>  static inline unsigned long sugov_apply_dvfs_headroom(unsigned long util,  int cpu)
>  {
> -	unsigned long update_headroom, waiting_headroom;
> +	unsigned long update_headroom, waiting_headroom, prev_util;
>  	struct rq *rq = cpu_rq(cpu);
>  	u64 delay;
>  
> +	prev_util = per_cpu(last_update_util, cpu);
> +	per_cpu(last_update_util, cpu) = util;
> +
> +	if (util < prev_util)
> +		return util;
> +
>  	/*
>  	 * What is the possible worst case scenario for updating util_avg, ctx
>  	 * switch or TICK?
> -- 
> 2.34.1
> 

Hmm, after the changes in "sched: cpufreq: Remove magic 1.25 headroom from
sugov_apply_dvfs_headroom()", won't sugov_apply_dvfs_headroom() already decay
the headroom gracefully in step with the decaying util? I suspect that abruptly
killing the headroom entirely could be premature depending on the workload, and
lead to util bouncing back up due to the time dilation effect you described in
the cover letter.

Cheers,
Sultan