[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <99a6d0e2-5b6e-4cd9-8bd5-e7f24a9a14f0@arm.com>
Date: Wed, 16 Apr 2025 13:26:35 +0100
From: Christian Loehle <christian.loehle@....com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Mario Limonciello <mario.limonciello@....com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Sultan Alsawaf <sultan@...neltoast.com>,
Peter Zijlstra <peterz@...radead.org>,
Valentin Schneider <vschneid@...hat.com>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH v2 3/6] cpufreq/sched: Set need_freq_update in
ignore_dl_rate_limit()
On 4/15/25 11:00, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Notice that ignore_dl_rate_limit() need not piggy back on the
> limits_changed handling to achieve its goal (which is to enforce a
> frequency update before its due time).
>
> Namely, if sugov_should_update_freq() is updated to check
> sg_policy->need_freq_update and return 'true' if it is set when
> sg_policy->limits_changed is not set, ignore_dl_rate_limit() may
> set the former directly instead of setting the latter, so it can
> avoid hitting the memory barrier in sugov_should_update_freq().
>
> Update the code accordingly.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Much cleaner now, thanks!
Reviewed-by: Christian Loehle <christian.loehle@....com>
> ---
> kernel/sched/cpufreq_schedutil.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -96,6 +96,9 @@
> smp_mb();
>
> return true;
> + } else if (sg_policy->need_freq_update) {
> + /* ignore_dl_rate_limit() wants a new frequency to be found. */
> + return true;
> }
>
> delta_ns = time - sg_policy->last_freq_update_time;
> @@ -388,7 +391,7 @@
> 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)
> - WRITE_ONCE(sg_cpu->sg_policy->limits_changed, true);
> + sg_cpu->sg_policy->need_freq_update = true;
> }
>
> static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu,
>
>
>
Powered by blists - more mailing lists