From: Peter Zijlstra <peterz@infradead.org>
To: Qais Yousef <qais.yousef@arm.com>
Cc: Ingo Molnar <mingo@kernel.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Valentin Schneider <valentin.schneider@arm.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sched/uclamp: Fix rq->uclamp_max not set on first enqueue
Date: Sat, 4 Dec 2021 10:43:49 +0100 [thread overview]
Message-ID: <20211204094349.GP16608@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20211202112033.1705279-1-qais.yousef@arm.com>
On Thu, Dec 02, 2021 at 11:20:33AM +0000, Qais Yousef wrote:
> Commit d81ae8aac85c ("sched/uclamp: Fix initialization of struct
> uclamp_rq") introduced a bug where uclamp_max of the rq is not reset to
> match the woken up task's uclamp_max when the rq is idle.
>
> The code was relying on rq->uclamp_max initialized to zero, so on first
> enqueue
>
> static inline void uclamp_rq_inc_id(struct rq *rq, struct task_struct *p,
> enum uclamp_id clamp_id)
> {
> ...
>
> if (uc_se->value > READ_ONCE(uc_rq->value))
> WRITE_ONCE(uc_rq->value, uc_se->value);
> }
>
> was actually resetting it. But since commit d81ae8aac85c changed the
> default to 1024, this no longer works. And since rq->uclamp_flags is
> also initialized to 0, neither above code path nor uclamp_idle_reset()
> update the rq->uclamp_max on first wake up from idle.
>
> This is only visible from first wake up(s) until the first dequeue to
> idle after enabling the static key. And it only matters if the
> uclamp_max of this task is < 1024 since only then its uclamp_max will be
> effectively ignored.
>
> Fix it by properly initializing rq->uclamp_flags = UCLAMP_FLAG_IDLE to
> ensure uclamp_idle_reset() is called which then will update the rq
> uclamp_max value as expected.
>
> Fixes: d81ae8aac85c ("sched/uclamp: Fix initialization of struct uclamp_rq")
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> Reviewed-by: Valentin Schneider <Valentin.Schneider@arm.com>
> Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Thanks!
next prev parent reply other threads:[~2021-12-04 9:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-02 11:20 [PATCH v2] sched/uclamp: Fix rq->uclamp_max not set on first enqueue Qais Yousef
2021-12-04 9:43 ` Peter Zijlstra [this message]
2021-12-04 13:40 ` [tip: sched/urgent] " tip-bot2 for Qais Yousef
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=20211204094349.GP16608@worktop.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=qais.yousef@arm.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.guittot@linaro.org \
/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.