Messages in this thread Patch in this message | | | From | Kirill Tkhai <> | Subject | Re: [PATCH] sched: reset sched_entity depth on changing parent | Date | Fri, 24 Oct 2014 21:18:42 +0400 |
| |
24.10.2014, 19:58, "Peter Zijlstra" <peterz@infradead.org>: > On Fri, Oct 24, 2014 at 11:07:46AM -0400, Burke Libbey wrote: >> From 2014-02-15: https://lkml.org/lkml/2014/2/15/217 >> >> This issue was reported and patched, but it still occurs in some situations on >> newer kernel versions. >> >> [2249353.328452] BUG: unable to handle kernel NULL pointer dereference at 0000000000000150 >> [2249353.336528] IP: [<ffffffff810b1cf7>] check_preempt_wakeup+0xe7/0x210 >> >> se.parent gets out of sync with se.depth, causing a panic when the algorithm in >> find_matching_se assumes they are correct. This patch forces se.depth to be >> updated every time se.parent is, so they can no longer become desync'd. >> >> CC: Ingo Molnar <mingo@kernel.org> >> CC: Peter Zijlstra <peterz@infradead.org> >> Signed-off-by: Burke Libbey <burke.libbey@shopify.com> >> --- >> >> I haven't been able to isolate the problem. Though I'm pretty confident this >> fixes the issue I've been having, I have not been able to prove it. > > So this isn't correct, switching rq should not change depth. I suspect > you're just papering over the issue by frequently resetting the value, > which simply narrows the race window.
Just a hypothesis.
I was seeking a places where task_group of a task may change. I can't understand how changing of parent's cgroup during fork() applies to a child.
Child's cgroup is the same as parent's after dup_task_struct(). The only function changing task_group is sched_move_task(), but we do not call it between dup_task_struct() and wake_up_new_task(). Shouldn't we do something like this?
(compile tested only) --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index cc18694..0ccbbdb 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7833,6 +7833,11 @@ static void cpu_cgroup_css_offline(struct cgroup_subsys_state *css) sched_offline_group(tg); } +static void cpu_cgroup_fork(struct task_struct *task) +{ + sched_move_task(task); +} + static int cpu_cgroup_can_attach(struct cgroup_subsys_state *css, struct cgroup_taskset *tset) { @@ -8205,6 +8210,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { .css_free = cpu_cgroup_css_free, .css_online = cpu_cgroup_css_online, .css_offline = cpu_cgroup_css_offline, + .fork = cpu_cgroup_fork, .can_attach = cpu_cgroup_can_attach, .attach = cpu_cgroup_attach, .exit = cpu_cgroup_exit,
Or we just should set tsk->sched_task_group? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |