From: Qais Yousef <qyousef@layalina.io>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Wyes Karny <wkarny@gmail.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Ingo Molnar <mingo@kernel.org>,
linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Juri Lelli <juri.lelli@redhat.com>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Valentin Schneider <vschneid@redhat.com>
Subject: Re: [GIT PULL] Scheduler changes for v6.8
Date: Sun, 14 Jan 2024 15:12:50 +0000 [thread overview]
Message-ID: <20240114151250.5wfexq44o3mdm3nh@airbuntu> (raw)
In-Reply-To: <CAKfTPtCz+95dR38c_u6_7JbkVt=czj5N2dKYVV-zk9dgbk16hw@mail.gmail.com>
On 01/14/24 14:03, Vincent Guittot wrote:
> Thanks for the trace. It was really helpful and I think that I got the
> root cause.
>
> The problem comes from get_capacity_ref_freq() which returns current
> freq when arch_scale_freq_invariant() is not enable, and the fact that
> we apply map_util_perf() earlier in the path now which is then capped
> by max capacity.
>
> Could you try the below ?
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index e420e2ee1a10..611c621543f4 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -133,7 +133,7 @@ unsigned long get_capacity_ref_freq(struct
> cpufreq_policy *policy)
> if (arch_scale_freq_invariant())
> return policy->cpuinfo.max_freq;
>
> - return policy->cur;
> + return policy->cur + policy->cur >> 2;
> }
>
> /**
Is this a test patch or a proper fix? I can't see it being the latter. It seems
the current logic fails when util is already 1024, and I think we're trying to
fix the invariance issue too late.
Is the problem that we can't read policy->cur in the scheduler to fix the util
while it's being updated that's why it's done here in this case?
If this is the problem, shouldn't the logic be if util is max then always go to
max frequency? I don't think we have enough info to correct the invariance here
IIUC. All we can see the system is saturated at this frequency and whether
a small jump or a big jump is required is hard to tell.
Something like this
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 95c3c097083e..473d0352030b 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -164,8 +164,12 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
struct cpufreq_policy *policy = sg_policy->policy;
unsigned int freq;
- freq = get_capacity_ref_freq(policy);
- freq = map_util_freq(util, freq, max);
+ if (util != max) {
+ freq = get_capacity_ref_freq(policy);
+ freq = map_util_freq(util, freq, max);
+ } else {
+ freq = policy->cpuinfo.max_freq;
+ }
if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
return sg_policy->next_freq;
next prev parent reply other threads:[~2024-01-14 15:12 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-28 12:23 [GIT PULL] Scheduler changes for v6.7 Ingo Molnar
2023-10-30 23:50 ` pr-tracker-bot
2024-01-08 14:07 ` [GIT PULL] Scheduler changes for v6.8 Ingo Molnar
2024-01-09 4:04 ` pr-tracker-bot
2024-01-10 22:19 ` Linus Torvalds
2024-01-10 22:41 ` Linus Torvalds
2024-01-10 22:57 ` Linus Torvalds
2024-01-11 8:11 ` Vincent Guittot
2024-01-11 17:45 ` Linus Torvalds
2024-01-11 17:53 ` Linus Torvalds
2024-01-11 18:16 ` Vincent Guittot
2024-01-12 14:23 ` Dietmar Eggemann
2024-01-12 16:58 ` Vincent Guittot
2024-01-12 18:18 ` Qais Yousef
2024-01-12 19:03 ` Vincent Guittot
2024-01-12 20:30 ` Linus Torvalds
2024-01-12 20:49 ` Linus Torvalds
2024-01-12 21:04 ` Linus Torvalds
2024-01-13 1:04 ` Qais Yousef
2024-01-13 1:24 ` Linus Torvalds
2024-01-13 1:31 ` Linus Torvalds
2024-01-13 10:47 ` Vincent Guittot
2024-01-13 18:33 ` Qais Yousef
2024-01-13 18:37 ` Qais Yousef
2024-01-11 11:09 ` [GIT PULL] scheduler fixes Ingo Molnar
2024-01-11 13:04 ` Vincent Guittot
2024-01-11 20:48 ` [PATCH] Revert "sched/cpufreq: Rework schedutil governor performance estimation" and dependent commit Ingo Molnar
2024-01-11 22:22 ` Vincent Guittot
2024-01-12 18:24 ` Ingo Molnar
2024-01-12 18:26 ` [GIT PULL] Scheduler changes for v6.8 Ingo Molnar
2024-01-14 9:12 ` Wyes Karny
2024-01-14 11:18 ` Vincent Guittot
2024-01-14 12:37 ` Wyes Karny
2024-01-14 13:02 ` Dietmar Eggemann
2024-01-14 13:05 ` Vincent Guittot
2024-01-14 13:03 ` Vincent Guittot
2024-01-14 15:12 ` Qais Yousef [this message]
2024-01-14 15:20 ` Vincent Guittot
2024-01-14 19:58 ` Qais Yousef
2024-01-14 23:37 ` Qais Yousef
2024-01-15 6:25 ` Wyes Karny
2024-01-15 11:59 ` Qais Yousef
2024-01-15 8:21 ` Vincent Guittot
2024-01-15 12:09 ` Qais Yousef
2024-01-15 13:26 ` Vincent Guittot
2024-01-15 14:03 ` Dietmar Eggemann
2024-01-15 15:26 ` Vincent Guittot
2024-01-15 20:05 ` Dietmar Eggemann
2024-01-15 8:42 ` David Laight
2024-01-14 18:11 ` Wyes Karny
2024-01-14 18:18 ` Vincent Guittot
2024-01-11 9:33 ` Ingo Molnar
2024-01-11 11:14 ` [tip: sched/urgent] Revert "sched/cpufreq: Rework schedutil governor performance estimation" and dependent commits tip-bot2 for Ingo Molnar
2024-01-11 20:55 ` [tip: sched/urgent] Revert "sched/cpufreq: Rework schedutil governor performance estimation" and dependent commit tip-bot2 for Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240114151250.5wfexq44o3mdm3nh@airbuntu \
--to=qyousef@layalina.io \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=wkarny@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.