linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Don <joshdon@google.com>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Paul Turner <pjt@google.com>,
	David Rientjes <rientjes@google.com>,
	Oleg Rombakh <olegrom@google.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Steve Sistare <steven.sistare@oracle.com>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sched: cgroup SCHED_IDLE support
Date: Tue, 15 Jun 2021 16:30:43 -0700	[thread overview]
Message-ID: <CABk29NvX8-kygVSB2ePNUsnebThyijrSfjxcEPft9kUfNm01cQ@mail.gmail.com> (raw)
In-Reply-To: <f48b5233-ce60-7e1a-02e6-1bfbcc852271@arm.com>

On Tue, Jun 15, 2021 at 3:07 AM Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
>
> On 12/06/2021 01:34, Josh Don wrote:
> > On Fri, Jun 11, 2021 at 9:43 AM Dietmar Eggemann
> > <dietmar.eggemann@arm.com> wrote:
> >>
> >> On 10/06/2021 21:14, Josh Don wrote:

[snip]

> > Setting cpu.idle carries additional properties in addition to just the
> > weight. Currently, it primarily includes (a) special wakeup preemption
> > handling, and (b) contribution to idle_h_nr_running for the purpose of
> > marking a cpu as a sched_idle_cpu(). Essentially, the current
> > SCHED_IDLE mechanics. I've also discussed with Peter a potential
> > extension to SCHED_IDLE to manipulate vruntime.
>
> Right, I forgot about (b).
>
> But IMHO, (a) could be handled with this special tg->shares value for
> SCHED_IDLE.
>
> If there would be a way to open up `cpu.weight`, `cpu.weight.nice` (and
> `cpu,shares` for v1) to take a special value for SCHED_IDLE, then you
> won't need cpu.idle.
> And you could handle the functionality from sched_group_set_idle()
> directly in sched_group_set_shares().
> In this case sched_group_set_shares() wouldn't have to be rejected on an
> idle tg.
> A tg would just become !idle by writing a different cpu.weight value.
> Currently, if you !idle a tg it gets the default NICE_0_LOAD.
>
> I guess cpu.weight [1, 10000] would be easy, 0 could be taken for that
> and mapped into weight = WEIGHT_IDLEPRIO (3, 3072) to call
> sched_group_set_shares(..., scale_load(weight).
> cpu.weight = 1 maps to (10, 10240)
>
> cpu.weight.nice [-20, 19] would be already more complicated, 20?
>
> And for cpu.shares [2, 2 << 18] 0 could be used. The issue here is that
> WEIGHT_IDLEPRIO (3, 3072) is a valid value already for shares.
>
> > We set the cgroup weight here, since by definition SCHED_IDLE entities
> > have the least scheduling weight. From the perspective of your
> > question, the analogous statement for tasks would be that we set task
> > weight to the min when doing setsched(SCHED_IDLE), even though we
> > already have a renice mechanism.
>
> I agree. `cpu.idle = 1` is like setting the task policy to SCHED_IDLE.
> And there is even the `cpu.weight.nice` to support the `task - tg`
> analogy on nice values.
>
> I'm just wondering if integrating this into `cpu.weight` and friends
> would be better to make the code behind this easier to grasp.

Might be confusing that we manipulate cgroup SCHED_IDLE via special
weight/weight.nice values, whereas task SCHED_IDLE is manipulated via
setsched (and we would allow nice 20 for cgroups but not for tasks,
for example).

More importantly, I think it would be better to avoid unintentional
side effects caused by overloading the weight interfaces. Thoughts on
that?

Another (less compelling) reason for the separate interface is that it
allows further extension. For example, negative values in cpu.idle
could indicate increasingly more latency-sensitive cgroups, which
could benefit from better wakeup preemption, entity placement, etc.

  reply	other threads:[~2021-06-15 23:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08 23:11 [PATCH] sched: cgroup SCHED_IDLE support Josh Don
2021-06-10 12:53 ` Dietmar Eggemann
2021-06-10 19:14   ` Josh Don
2021-06-11 16:43     ` Dietmar Eggemann
2021-06-11 23:34       ` Josh Don
2021-06-15 10:06         ` Dietmar Eggemann
2021-06-15 23:30           ` Josh Don [this message]
2021-06-25  9:24           ` Peter Zijlstra
2021-06-16 15:42 ` Tejun Heo
2021-06-17  1:01   ` Josh Don
2021-06-26  9:57     ` Tejun Heo
2021-06-29  4:57       ` Josh Don
2021-06-25  8:08   ` Peter Zijlstra
2021-06-26 10:06     ` Tejun Heo
2021-06-26 11:42     ` Rik van Riel
2021-06-25  8:14 ` Peter Zijlstra
2021-06-26  0:18   ` Josh Don
2021-06-25  8:20 ` Peter Zijlstra
2021-06-26  0:35   ` Josh Don

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=CABk29NvX8-kygVSB2ePNUsnebThyijrSfjxcEPft9kUfNm01cQ@mail.gmail.com \
    --to=joshdon@google.com \
    --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@redhat.com \
    --cc=olegrom@google.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=steven.sistare@oracle.com \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@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 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).