From: Patrick Bellasi <patrick.bellasi@matbug.net>
To: Quentin Perret <qperret@google.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
Qais Yousef <qais.yousef@arm.com>, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Steven Rostedt <rostedt@goodmis.org>,
Luis Chamberlain <mcgrof@kernel.org>,
Kees Cook <keescook@chromium.org>,
Iurii Zaikin <yzaikin@google.com>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
valentin.schneider@arm.com, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, kernel-team@android.com
Subject: Re: [PATCH] sched/rt: Add a new sysctl to control uclamp_util_min
Date: Wed, 8 Jan 2020 20:16:00 +0100 [thread overview]
Message-ID: <20200108191600.GC9635@darkstar> (raw)
In-Reply-To: <20200108095108.GA153171@google.com>
On 08-Jan 09:51, Quentin Perret wrote:
> On Tuesday 07 Jan 2020 at 20:30:36 (+0100), Dietmar Eggemann wrote:
> > On 07/01/2020 14:42, Quentin Perret wrote:
> > > Hi Qais,
> > >
> > > On Friday 20 Dec 2019 at 16:48:38 (+0000), Qais Yousef wrote:
> >
> > [...]
> >
> > >> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> > >> index e591d40fd645..19572dfc175b 100644
> > >> --- a/kernel/sched/rt.c
> > >> +++ b/kernel/sched/rt.c
> > >> @@ -2147,6 +2147,12 @@ static void pull_rt_task(struct rq *this_rq)
> > >> */
> > >> static void task_woken_rt(struct rq *rq, struct task_struct *p)
> > >> {
> > >> + /*
> > >> + * When sysctl_sched_rt_uclamp_util_min value is changed by the user,
> > >> + * we apply any new value on the next wakeup, which is here.
> > >> + */
> > >> + uclamp_rt_sync_default_util_min(p);
> > >
> > > The task has already been enqueued and sugov has been called by then I
> > > think, so this is a bit late. You could do that in uclamp_rq_inc() maybe?
No, the above sync should neven be done.
For CFS tasks is currenly working by computing the effective clamp
from core.c::enqueue_task() before calling into the scheduling class
callback.
We should use a similar approach for RT tasks thus avoiding rt.c
specific code and ensuring correct effective values are _aggregated_
out of task's _requests_ and system-wide's _constraints_ before
calling sugov.
> > That's probably better.
> > Just to be sure ...we want this feature (an existing rt task gets its
> > UCLAMP_MIN value set when the sysctl changes)
No, that's not a feature. That's an hack I would avoid.
> > because there could be rt tasks running before the sysctl is set?
>
> Yeah, I was wondering the same thing, but I'd expect sysadmin to want
> this. We could change the min clamp of existing RT tasks in userspace
> instead, but given how simple Qais' lazy update code is, the in-kernel
> looks reasonable to me. No strong opinion, though.
>
> Thanks,
> Quentin
--
#include <best/regards.h>
Patrick Bellasi
next prev parent reply other threads:[~2020-01-08 19:16 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-20 16:48 [PATCH] sched/rt: Add a new sysctl to control uclamp_util_min Qais Yousef
2020-01-07 13:42 ` Quentin Perret
2020-01-07 19:30 ` Dietmar Eggemann
2020-01-08 9:51 ` Quentin Perret
2020-01-08 19:16 ` Patrick Bellasi [this message]
2020-01-09 11:36 ` Qais Yousef
2020-01-09 11:16 ` Qais Yousef
2020-01-09 11:12 ` Qais Yousef
2020-01-08 13:44 ` Peter Zijlstra
2020-01-08 19:08 ` Patrick Bellasi
2020-01-09 13:00 ` Qais Yousef
2020-01-10 13:39 ` Peter Zijlstra
2020-01-12 23:35 ` Qais Yousef
2020-01-10 13:42 ` Peter Zijlstra
2020-01-12 23:31 ` Qais Yousef
2020-01-08 18:56 ` Patrick Bellasi
2020-01-09 1:35 ` Valentin Schneider
2020-01-09 9:21 ` Patrick Bellasi
2020-01-09 13:38 ` Qais Yousef
2020-01-14 21:34 ` Qais Yousef
2020-01-22 10:19 ` Patrick Bellasi
2020-01-22 11:45 ` Qais Yousef
2020-01-22 12:44 ` Patrick Bellasi
2020-01-22 14:57 ` Qais Yousef
2020-01-09 13:15 ` 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=20200108191600.GC9635@darkstar \
--to=patrick.bellasi@matbug.net \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=keescook@chromium.org \
--cc=kernel-team@android.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=qais.yousef@arm.com \
--cc=qperret@google.com \
--cc=rostedt@goodmis.org \
--cc=valentin.schneider@arm.com \
--cc=vincent.guittot@linaro.org \
--cc=yzaikin@google.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).