* [PATCH] sched/core: fix sched_rt_global_validate @ 2014-01-31 16:43 Juri Lelli 2014-02-03 9:53 ` Henrik Austad 0 siblings, 1 reply; 4+ messages in thread From: Juri Lelli @ 2014-01-31 16:43 UTC (permalink / raw) To: peterz; +Cc: mingo, linux-kernel, Juri Lelli 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; return 0; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/core: fix sched_rt_global_validate 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 0 siblings, 1 reply; 4+ messages in thread From: Henrik Austad @ 2014-02-03 9:53 UTC (permalink / raw) To: Juri Lelli; +Cc: peterz, mingo, linux-kernel 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? -- Henrik Austad ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/core: fix sched_rt_global_validate 2014-02-03 9:53 ` Henrik Austad @ 2014-02-03 10:14 ` Juri Lelli 2014-02-03 13:31 ` Henrik Austad 0 siblings, 1 reply; 4+ messages in thread From: Juri Lelli @ 2014-02-03 10:14 UTC (permalink / raw) To: Henrik Austad; +Cc: peterz, mingo, linux-kernel 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). 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). Makes sense? Thanks, - Juri ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/core: fix sched_rt_global_validate 2014-02-03 10:14 ` Juri Lelli @ 2014-02-03 13:31 ` Henrik Austad 0 siblings, 0 replies; 4+ messages in thread From: Henrik Austad @ 2014-02-03 13:31 UTC (permalink / raw) To: Juri Lelli; +Cc: peterz, mingo, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-03 13:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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).