[sched] d50dde5a1: LTP sched_setparam* TFAILs

[sched] d50dde5a1: LTP sched_setparam* TFAILs

[Fengguang Wu]

Hi Dario,

We notice the below LTP failures since commit d50dde5a1 ("sched: Add
new scheduler syscalls to support an extended scheduling parameters
ABI") in tip tree's sched/core branch:

ltp.sched_setparam02.1.TFAIL
ltp.sched_setparam02.2.TFAIL
ltp.sched_setparam02.3.TFAIL
ltp.sched_setparam03.1.TFAIL

It seems that the LKP test failures are somehow expected due to the
change of ABI. However it does mean that these test cases will need to
be updated.

Thanks,
Fengguang

Re: [sched] d50dde5a1: LTP sched_setparam* TFAILs

[Peter Zijlstra]

On Wed, Jan 15, 2014 at 08:47:39PM +0800, Fengguang Wu wrote:
> Hi Dario,
> 
> We notice the below LTP failures since commit d50dde5a1 ("sched: Add
> new scheduler syscalls to support an extended scheduling parameters
> ABI") in tip tree's sched/core branch:
> 
> ltp.sched_setparam02.1.TFAIL
> ltp.sched_setparam02.2.TFAIL
> ltp.sched_setparam02.3.TFAIL
> ltp.sched_setparam03.1.TFAIL
> 
> It seems that the LKP test failures are somehow expected due to the
> change of ABI. However it does mean that these test cases will need to
> be updated.

Nah, the old API should keep working. Lemme find where this LTP stuff
lives.

Re: [sched] d50dde5a1: LTP sched_setparam* TFAILs

[Peter Zijlstra]

On Wed, Jan 15, 2014 at 08:47:39PM +0800, Fengguang Wu wrote:
> Hi Dario,
> 
> We notice the below LTP failures since commit d50dde5a1 ("sched: Add
> new scheduler syscalls to support an extended scheduling parameters
> ABI") in tip tree's sched/core branch:
> 
> ltp.sched_setparam02.1.TFAIL
> ltp.sched_setparam02.2.TFAIL
> ltp.sched_setparam02.3.TFAIL
> ltp.sched_setparam03.1.TFAIL
> 
> It seems that the LKP test failures are somehow expected due to the
> change of ABI. However it does mean that these test cases will need to
> be updated.

So with LTP from https://github.com/linux-test-project/ltp.git
and a tip/master build, I get:

root@westmere:/usr/local/src/ltp/testcases/kernel/syscalls/sched_setscheduler# ./sched_setscheduler01 
sched_setscheduler01    1  TPASS  :  expected failure - errno = 3 : No such process
sched_setscheduler01    2  TPASS  :  expected failure - errno = 22 : Invalid argument
sched_setscheduler01    3  TPASS  :  expected failure - errno = 14 : Bad address
sched_setscheduler01    4  TPASS  :  expected failure - errno = 22 : Invalid argument
root@westmere:/usr/local/src/ltp/testcases/kernel/syscalls/sched_setscheduler# ./sched_setscheduler02 
sched_setscheduler02    1  TPASS  :  got EPERM

Re: [sched] d50dde5a1: LTP sched_setparam* TFAILs

[Fengguang Wu]

On Wed, Jan 15, 2014 at 02:09:38PM +0100, Peter Zijlstra wrote:
> On Wed, Jan 15, 2014 at 08:47:39PM +0800, Fengguang Wu wrote:
> > Hi Dario,
> > 
> > We notice the below LTP failures since commit d50dde5a1 ("sched: Add
> > new scheduler syscalls to support an extended scheduling parameters
> > ABI") in tip tree's sched/core branch:
> > 
> > ltp.sched_setparam02.1.TFAIL
> > ltp.sched_setparam02.2.TFAIL
> > ltp.sched_setparam02.3.TFAIL
> > ltp.sched_setparam03.1.TFAIL
> > 
> > It seems that the LKP test failures are somehow expected due to the
> > change of ABI. However it does mean that these test cases will need to
> > be updated.
> 
> So with LTP from https://github.com/linux-test-project/ltp.git
> and a tip/master build, I get:
> 
> root@westmere:/usr/local/src/ltp/testcases/kernel/syscalls/sched_setscheduler# ./sched_setscheduler01 
> sched_setscheduler01    1  TPASS  :  expected failure - errno = 3 : No such process
> sched_setscheduler01    2  TPASS  :  expected failure - errno = 22 : Invalid argument
> sched_setscheduler01    3  TPASS  :  expected failure - errno = 14 : Bad address
> sched_setscheduler01    4  TPASS  :  expected failure - errno = 22 : Invalid argument
> root@westmere:/usr/local/src/ltp/testcases/kernel/syscalls/sched_setscheduler# ./sched_setscheduler02 
> sched_setscheduler02    1  TPASS  :  got EPERM

It's sched_setparam02 that TFAIL, not sched_setscheduler02. :)

Thanks,
Fengguang

Re: [sched] d50dde5a1: LTP sched_setparam* TFAILs

[Peter Zijlstra]

On Wed, Jan 15, 2014 at 09:26:09PM +0800, Fengguang Wu wrote:
> It's sched_setparam02 that TFAIL, not sched_setscheduler02. :)

D'0h!

[PATCH] sched: Fixup scheduler syscall LTP fails

[Peter Zijlstra]

Subject: sched: Fixup scheduler syscall LTP fails

Wu reported:

> ltp.sched_setparam02.1.TFAIL
> ltp.sched_setparam02.2.TFAIL
> ltp.sched_setparam02.3.TFAIL
> ltp.sched_setparam03.1.TFAIL

There were 2 things wrong; firstly __setscheduler() failed on
sched_setparam()'s policy = -1, fix that by reading from p->policy in
that case.

Secondly, getparam() (and getattr()) would still report !0
sched_priority for !FIFO/RR tasks after having been such. So
unconditionally set p->rt_priority.

Fixes: d50dde5a10f3 ("sched: Add new scheduler syscalls to support an extended scheduling parameters ABI")
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/core.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 0326c06953eb..50e99f2cf98b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3183,15 +3183,23 @@ static void __setscheduler(struct rq *rq, struct task_struct *p,
 {
 	int policy = attr->sched_policy;
 
+	if (policy == -1) /* setparam */
+		policy = p->policy;
+
 	p->policy = policy;
 
 	if (dl_policy(policy))
 		__setparam_dl(p, attr);
-	else if (rt_policy(policy))
-		p->rt_priority = attr->sched_priority;
-	else
+	else if (fair_policy(policy))
 		p->static_prio = NICE_TO_PRIO(attr->sched_nice);
 
+	/*
+	 * __sched_setscheduler() ensures attr->sched_priority == 0 when
+	 * !rt_policy. Always setting this ensures that things like
+	 * getparam()/getattr() don't report silly values for !rt tasks.
+	 */
+	p->rt_priority = attr->sched_priority;
+
 	p->normal_prio = normal_prio(p);
 	p->prio = rt_mutex_getprio(p);
 

[tip:sched/core] sched: Fix up scheduler syscall LTP fails

[tip-bot for Peter Zijlstra]

Commit-ID:  39fd8fd22b3224ec6819d33b3e34ae4da6a35f05
Gitweb:     http://git.kernel.org/tip/39fd8fd22b3224ec6819d33b3e34ae4da6a35f05
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 15 Jan 2014 16:33:20 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 16 Jan 2014 09:27:15 +0100

sched: Fix up scheduler syscall LTP fails

Wu reported LTP failures:

  > ltp.sched_setparam02.1.TFAIL
  > ltp.sched_setparam02.2.TFAIL
  > ltp.sched_setparam02.3.TFAIL
  > ltp.sched_setparam03.1.TFAIL

There were 2 things wrong; firstly __setscheduler() failed on
sched_setparam()'s policy = -1, fix that by reading from p->policy in
that case.

Secondly, getparam() (and getattr()) would still report !0
sched_priority for !FIFO/RR tasks after having been such. So
unconditionally set p->rt_priority.

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@gmail.com>
Cc: Dario Faggioli <raistlin@linux.it>
Fixes: d50dde5a10f3 ("sched: Add new scheduler syscalls to support an extended scheduling parameters ABI")
Link: http://lkml.kernel.org/r/20140115153320.GH31570@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c1b3d7e..e9212eb 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3172,15 +3172,23 @@ static void __setscheduler(struct rq *rq, struct task_struct *p,
 {
 	int policy = attr->sched_policy;
 
+	if (policy == -1) /* setparam */
+		policy = p->policy;
+
 	p->policy = policy;
 
 	if (dl_policy(policy))
 		__setparam_dl(p, attr);
-	else if (rt_policy(policy))
-		p->rt_priority = attr->sched_priority;
-	else
+	else if (fair_policy(policy))
 		p->static_prio = NICE_TO_PRIO(attr->sched_nice);
 
+	/*
+	 * __sched_setscheduler() ensures attr->sched_priority == 0 when
+	 * !rt_policy. Always setting this ensures that things like
+	 * getparam()/getattr() don't report silly values for !rt tasks.
+	 */
+	p->rt_priority = attr->sched_priority;
+
 	p->normal_prio = normal_prio(p);
 	p->prio = rt_mutex_getprio(p);