From: Henrik Austad <henrik@austad.us>
To: Juri Lelli <juri.lelli@gmail.com>
Cc: peterz@infradead.org, mingo@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/core: fix sched_rt_global_validate
Date: Mon, 3 Feb 2014 14:31:02 +0100 [thread overview]
Message-ID: <20140203133102.GA13556@austad.us> (raw)
In-Reply-To: <52EF6BED.7050001@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2087 bytes --]
On Mon, Feb 03, 2014 at 11:14:05AM +0100, Juri Lelli wrote:
> On 02/03/2014 10:53 AM, Henrik Austad wrote:
> > On Fri, Jan 31, 2014 at 05:43:27PM +0100, Juri Lelli wrote:
> >> Don't compare sysctl_sched_rt_runtime against sysctl_sched_rt_period if
> >> the former is equal to RUNTIME_INF, otherwise disabling -rt bandwidth
> >> management always fails.
> >>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> Signed-off-by: Juri Lelli <juri.lelli@gmail.com>
> >> ---
> >> kernel/sched/core.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >> index 210a12a..5c0a304 100644
> >> --- a/kernel/sched/core.c
> >> +++ b/kernel/sched/core.c
> >> @@ -7477,7 +7477,8 @@ static int sched_rt_global_validate(void)
> >> if (sysctl_sched_rt_period <= 0)
> >> return -EINVAL;
> >>
> >> - if (sysctl_sched_rt_runtime > sysctl_sched_rt_period)
> >> + if ((sysctl_sched_rt_runtime != RUNTIME_INF) &&
> >> + (sysctl_sched_rt_runtime > sysctl_sched_rt_period))
> >> return -EINVAL;
> >
> > Won't this be caught by the test above?
> >
> > #define RUNTIME_INF ((u64)~0ULL)
> >
> > which means that if sysctl_sched_rt_runtime is set to RUNTIME_INF, it will
> > trigger on the previous test, and the first part of this test will always
> > be true.
> >
> > Or have I suffered catastrophic monday-morning braindamage?
> >
>
> As I understand it. When you do
>
> echo -1 > /proc/sys/kernel/sched_rt_runtime_us
>
> sysctl_sched_rt_runtime is actually set to -1 (being an int).
Yes.
> But then you compare it against and unsigned int, so the cast converts it to
> actually be RUNTIME_INF, and thus greater than sysctl_sched_rt_period (so the
> function returns -EINVAL, while you'd want it to return 0, as you are disabling
> -rt throttling).
Ah, yes, it comes down to my early-monday brain hemorrhage, mixing up
rt_period and rt_runtime. My apologies!
> Makes sense?
Yes it does.
--
Henrik Austad
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
prev parent reply other threads:[~2014-02-03 13:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-31 16:43 [PATCH] sched/core: fix sched_rt_global_validate Juri Lelli
2014-02-03 9:53 ` Henrik Austad
2014-02-03 10:14 ` Juri Lelli
2014-02-03 13:31 ` Henrik Austad [this message]
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=20140203133102.GA13556@austad.us \
--to=henrik@austad.us \
--cc=juri.lelli@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.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.