From: Juri Lelli <juri.lelli@redhat.com>
To: Peng Liu <iwtbavbm@gmail.com>
Cc: mingo@redhat.com, peterz@infradead.org,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
bristot@redhat.com, valentin.schneider@arm.com,
raistlin@linux.it, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/deadline: Fix sched_dl_global_validate()
Date: Wed, 16 Sep 2020 11:42:03 +0200 [thread overview]
Message-ID: <20200916094203.GA6652@localhost.localdomain> (raw)
In-Reply-To: <20200915152048.GA25835@iZj6chx1xj0e0buvshuecpZ>
Hi,
On 15/09/20 23:20, Peng Liu wrote:
> When user changes sched_rt_{runtime, period}_us, then
>
> sched_rt_handler()
> --> sched_dl_bandwidth_validate()
> {
> new_bw = global_rt_runtime()/global_rt_period();
>
> for_each_possible_cpu(cpu) {
> dl_b = dl_bw_of(cpu);
> if (new_bw < dl_b->total_bw)
> ret = -EBUSY;
> }
> }
>
> Under CONFIG_SMP, dl_bw is per root domain , but not per CPU,
> dl_b->total_bw is the allocated bandwidth of the whole root domain.
> we should compare dl_b->total_bw against cpus*new_bw, where 'cpus'
> is the number of CPUs of the root domain.
Hummm, I think you are right. Guess nobody played a lot with changing
global values. :-/
> Also, below annotation(in kernel/sched/sched.h) implied implementation
> only appeared in SCHED_DEADLINE v2[1], then deadline scheduler kept
> evolving till got merged(v9), but the annotation remains unchanged,
> meaningless and misleading, correct it.
>
> * With respect to SMP, the bandwidth is given on a per-CPU basis,
> * meaning that:
> * - dl_bw (< 100%) is the bandwidth of the system (group) on each CPU;
> * - dl_total_bw array contains, in the i-eth element, the currently
> * allocated bandwidth on the i-eth CPU.
>
> [1] https://lkml.org/lkml/2010/2/28/119
>
> Signed-off-by: Peng Liu <iwtbavbm@gmail.com>
> ---
> In fact, I'm not 100% sure that's a bug, since it's too 'obvious' and
> not newly introduced code.
>
> Also, the introduced #ifdef...#endif pairs look ugly, I have no idea
> how to eliminate them. Ideas and comments are welcome. Thanks.
Can't we just use dl_bw_cpus() ?
Thanks,
Juri
next prev parent reply other threads:[~2020-09-16 9:42 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-15 15:20 [PATCH] sched/deadline: Fix sched_dl_global_validate() Peng Liu
2020-09-16 9:42 ` Juri Lelli [this message]
2020-09-16 15:39 ` Peng Liu
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=20200916094203.GA6652@localhost.localdomain \
--to=juri.lelli@redhat.com \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=iwtbavbm@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=raistlin@linux.it \
--cc=rostedt@goodmis.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox