On Fri, Apr 03, 2015 at 03:51:17PM +0300, Konstantin Khlebnikov wrote:
> On 03.04.2015 15:41, Konstantin Khlebnikov wrote:
> >Pick_next_task_fair() must be sure that here is at least one runnable
> >task before calling put_prev_task(), but put_prev_task() can expire
> >last remains of cfs quota and throttle all currently runnable tasks.
> >As a result pick_next_task_fair() cannot find next task and crashes.
>
> Kernel crash looks like this:
>
> <1>[50288.719491] BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000038
> <1>[50288.719538] IP: [<ffffffff81097b8c>] set_next_entity+0x1c/0x80
> <4>[50288.720388] Call Trace:
> <4>[50288.720402] [<ffffffff8109ead8>] pick_next_task_fair+0x88/0x5d0
> <4>[50288.720479] [<ffffffff816b5b99>] __schedule+0xf9/0x7d0
Which set_next_entity() is that? There are 3 in pick_next_task_fair().
I have a vague suspicion its in the 'simple' code, please verify.
The thinking is that if it was the 'complex' pick_next_entity()
returning NULL we'd have exploded elsewhere, the cfs_rq iteration
would've wandered off into random memory and most likely exploded on
cfs_rq->curr.
Which too would suggest the check_cfs_rq_runtime() thing works just
fine, it send us to the simple code.
> >This patch leaves 1 in ->runtime_remaining when current assignation
> >expires and tries to refill it right after that. In the worst case
> >task will be scheduled once and throttled at the end of slice.
Which is a strange approach. If pick_next_task_fair() is borken, we
should fix that, no?
In any case, it appears to me that: 606dba2e2894 ("sched: Push
put_prev_task() into pick_next_task()") inverted the ->nr_running and
put_prev_task() statements.
If the above set_next_entity() is indeed the simple one, does the below
cure things?
---
kernel/sched/fair.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index fdae26eb7218..df72d61138a8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5176,12 +5176,11 @@ pick_next_task_fair(struct rq *rq, struct task_struct
*prev)
simple:
cfs_rq = &rq->cfs;
#endif
+ put_prev_task(rq, prev);
if (!cfs_rq->nr_running)
goto idle;
- put_prev_task(rq, prev);
-
do {
se = pick_next_entity(cfs_rq, NULL);
set_next_entity(cfs_rq, se);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
