From: Quentin Perret <qperret@google.com>
To: Qais Yousef <qais.yousef@arm.com>
Cc: mingo@redhat.com, peterz@infradead.org,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rickyiu@google.com, wvw@google.com, patrick.bellasi@matbug.net,
xuewen.yan94@gmail.com, linux-kernel@vger.kernel.org,
kernel-team@android.com
Subject: Re: [PATCH v3 1/3] sched: Fix UCLAMP_FLAG_IDLE setting
Date: Fri, 2 Jul 2021 11:54:07 +0000 [thread overview]
Message-ID: <YN7+X+5NviroSqZY@google.com> (raw)
In-Reply-To: <20210701175932.66hiwvuia4drs4yl@e107158-lin.cambridge.arm.com>
On Thursday 01 Jul 2021 at 18:59:32 (+0100), Qais Yousef wrote:
> On 07/01/21 15:20, Quentin Perret wrote:
> > > > Right or maybe we can just check that uclamp_id == UCLAMP_MAX here and
> > > > we should be good to go? That is, what about the below?
> > >
> > > Wouldn't it be better to do this from uclamp_idle_reset() then?
> >
> > That should work too, but clearing the flag outside of
> > uclamp_rq_inc_id() feels a little bit more robust to ordering
> > issues.
> >
> > Specifically, uclamp_rq_inc() has the following pattern:
> >
> > for_each_clamp_id(clamp_id)
> > uclamp_rq_inc_id(rq, p , clamp_id);
> >
> > if (rq->uclamp_flags & UCLAMP_FLAG_IDLE)
> > rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE;
> >
> > So, if we change this to clear the flag from
> > uclamp_rq_inc_id()->uclamp_idle_reset() then we'll have issues if
> > (for example) for_each_clamp_id()'s order changes in the future.
> > IOW, it feels cleaner to not create side effects in uclamp_rq_inc_id()
> > that impact the idle flag given that its very own behaviour depends on
> > the flag.
> >
> > WDYT?
>
> Do the clearing from outside the loop then to keep the pattern consistent?
Right, but I actually preferred doing it from here as we're under
task_rq_lock(), which means well behaved readers won't observe the flag
being transiently set. I could also refactor the locking, but oh well ...
> Anyway, I think there's no clear objective advantage. So I'll trust your
> judgement and promise not to complain with your final choice ;-)
:) Alrighty, I'll cook something.
Thanks!
Quentin
next prev parent reply other threads:[~2021-07-02 11:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-23 12:34 [PATCH v3 0/3] sched: A few uclamp fixes and tweaks Quentin Perret
2021-06-23 12:34 ` [PATCH v3 1/3] sched: Fix UCLAMP_FLAG_IDLE setting Quentin Perret
2021-06-30 14:58 ` Qais Yousef
2021-06-30 15:45 ` Quentin Perret
2021-07-01 10:07 ` Quentin Perret
2021-07-01 11:08 ` Qais Yousef
2021-07-01 12:43 ` Quentin Perret
2021-07-01 14:57 ` Qais Yousef
2021-07-01 15:20 ` Quentin Perret
2021-07-01 17:59 ` Qais Yousef
2021-07-02 11:54 ` Quentin Perret [this message]
2021-07-01 11:06 ` Qais Yousef
2021-06-23 12:34 ` [PATCH v3 2/3] sched: Skip priority checks with SCHED_FLAG_KEEP_PARAMS Quentin Perret
2021-06-30 16:01 ` Qais Yousef
2021-06-23 12:34 ` [PATCH v3 3/3] sched: Introduce RLIMIT_UCLAMP Quentin Perret
2021-07-01 10:50 ` Qais Yousef
2021-07-01 12:05 ` Quentin Perret
2021-07-01 17:52 ` Qais Yousef
2021-07-02 12:28 ` Quentin Perret
2021-07-08 11:36 ` Qais Yousef
2021-07-19 11:44 ` Quentin Perret
2021-07-26 16:22 ` 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=YN7+X+5NviroSqZY@google.com \
--to=qperret@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=patrick.bellasi@matbug.net \
--cc=peterz@infradead.org \
--cc=qais.yousef@arm.com \
--cc=rickyiu@google.com \
--cc=vincent.guittot@linaro.org \
--cc=wvw@google.com \
--cc=xuewen.yan94@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).