lkml.org 
[lkml]   [2014]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [BUG] [ tip/sched/core ] System unresponsive after booting

* Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

>
> Hi all,
>
> I use the tip/sched/core branch.
>
> After git pulling yesterday, my host is unresponsive after booting the OS.
>
> * It boots normally
> * It sends info to the console
> * The graphics does not work
> * The terminals show the prompt, I can enter the username but after
> pressing enter, it does not give the password prompt
> * sysrq works more or less, I can't get the process stack but it
> receives the command
>
> It is like no new process can be created.
>
> I have a dual Xeon processor E5325 (2 x 4 cores).
>
> After git bisecting, the following patch seems to introduce the bug.
>
> commit d50dde5a10f305253cbc3855307f608f8a3c5f73
> Author: Dario Faggioli <raistlin@linux.it>
> Date: Thu Nov 7 14:43:36 2013 +0100
>
> sched: Add new scheduler syscalls to support an extended
> scheduling parameters ABI
>
> Add the syscalls needed for supporting scheduling algorithms
> with extended scheduling parameters (e.g., SCHED_DEADLINE).
>
>
> [ ... ]
>
>
> Signed-off-by: Dario Faggioli <raistlin@linux.it>
> [ Rewrote to use sched_attr. ]
> Signed-off-by: Juri Lelli <juri.lelli@gmail.com>
> [ Removed sched_setscheduler2() for now. ]
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> Link: http://lkml.kernel.org/r/1383831828-15501-3-git-send-email-juri.lelli@gmail.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>

I checked this patch again, and noticed a few oddities:

1)

There's this change to __setscheduler():

-__setscheduler(struct rq *rq, struct task_struct *p, int policy, int prio)
+/* Actually do priority change: must hold pi & rq lock. */
+static void __setscheduler(struct rq *rq, struct task_struct *p,
+ const struct sched_attr *attr)
{
+ int policy = attr->sched_policy;
+
p->policy = policy;
- p->rt_priority = prio;
+
+ if (rt_policy(policy))
+ p->rt_priority = attr->sched_priority;
+ else
+ p->static_prio = NICE_TO_PRIO(attr->sched_nice);
+


doesnt this change keep p->rt_priority uninitialized in the
normalize_task() case?

I.e. rt_priority should still be set unconditionally. In the
SCHED_NORMAL case that will be a zero initialization.

2)

It's not clear why this change to __setscheduler() was done:

/*
* Allow unprivileged RT tasks to decrease priority:
*/
if (user && !capable(CAP_SYS_NICE)) {
+ if (fair_policy(policy)) {
+ if (!can_nice(p, attr->sched_nice))
+ return -EPERM;
+ }
+
if (rt_policy(policy)) {


3)

On ARM:

-#define __NR_syscalls (380)
+#define __NR_syscalls (384)

but:

#define __NR_finit_module (__NR_SYSCALL_BASE+379)
+#define __NR_sched_setattr (__NR_SYSCALL_BASE+380)
+#define __NR_sched_getattr (__NR_SYSCALL_BASE+381)

/*


Why is the syscall table increased by 4, while we only add 2 new
syscalls?

4)

In hindsight this patch should have been 3 patches or so:

- one that just mechanically extends __setscheduler() with an 'attr'
way to pass parameters

- one that adds whatever other desired changes to __setscheduler(),
with an explanation.

- one that adds the new syscalls.

Which would ease the debugging of such bugs.

Thanks,

Ingo


\
 
 \ /
  Last update: 2014-01-15 11:01    [W:0.158 / U:1.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site