Hang with fair cgroup scheduler (reproducer is attached.)
Hi, While I was testing 2.6.24-rc5-mm1's fair group scheduler (with cgroup), the system hangs. please confirm. it's reproducible on my box. My test program is attached. What happens: the system hangs. (panic ?) Environ: ia64/NUMA 8CPU systems. 4 cpus per node. How to reproduce: Compile attached one. # gcc -o reg reg.c Create group as following # mount -t cgroup none /opt/cgroup -o cpu # mkdir /opt/cgroup/group_1 # mkdir /opt/cgroup/group_2 And run attached program # ./reg 8 8 What 'reg' does; usage : reg A B C... This program forks child process and assign A of processes to group_1 B of processes to group_2 C of processes to group_3 kick and waitpid all and repeat. Thanks, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
Re: Hang with fair cgroup scheduler (reproducer is attached.)
(Cc:-ed other folks as well) * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > Hi, > > While I was testing 2.6.24-rc5-mm1's fair group scheduler (with cgroup), > the system hangs. please confirm. it's reproducible on my box. > > My test program is attached. > > What happens: > the system hangs. (panic ?) > > Environ: > ia64/NUMA 8CPU systems. 4 cpus per node. > > How to reproduce: > Compile attached one. > # gcc -o reg reg.c > Create group as following > # mount -t cgroup none /opt/cgroup -o cpu > # mkdir /opt/cgroup/group_1 > # mkdir /opt/cgroup/group_2 > > And run attached program > # ./reg 8 8 > > What 'reg' does; > usage : reg A B C... > This program forks child process and assign > A of processes to group_1 > B of processes to group_2 > C of processes to group_3 > kick and waitpid all and repeat. > > Thanks, > -Kame > #include <stdlib.h> > #include <stdio.h> > #include <strings.h> > #include <sys/types.h> > #include <unistd.h> > #include <sched.h> > #include <asm/intrinsics.h> > #include <sys/ipc.h> > #include <sys/shm.h> > #include <errno.h> > #include <sys/times.h> > > static char *shared; > #define MAX_PROCS 32 > #define SHMSIZE (16384) > > struct start_stop { > int go; > }; > > /* Assign PID to a group.... > * work as # echo PID > /opt/cgroup/group_%d/tasks > */ > void assign_to(int pid, int group) > { > FILE *fp; > char buf[32]; > > memset(buf, 0, sizeof(buf)); > sprintf(buf,"/opt/cgroup/group_%d/tasks",group); > fp = fopen(buf,"w"); > if (fp == NULL) { > perror("fopen"); > fprintf(stderr, "failed : fopen"); > exit(0); > } > fprintf(fp, "%d", pid); > fclose(fp); > printf("%d to %s\n", pid, buf); > } > > /* > * spin wait and go into small loop. > * # of loops are counted as score. > * This process's utime is recorded in times[id] > */ > int worker(int id) > { > struct start_stop *shared_flag; > > shared_flag = (struct start_stop*)shared; > do { > sched_yield(); > ia64_mf(); > } while (!shared_flag->go); > } > > /* > * If you want to assign.. > * 2 proces to group 1, 3 procs to group 2 -># ./a.out 2 3 > * 3 proces to group 1, 3 procs to group 2, 3 procs to group 3 > * -># ./a.out 3 3 3 > * Total 32 procs are supported. > */ > > int main(int argc, char *argv[]) > { > int nprocs; > int shmid, i; > struct start_stop *shared_flag; > int pids[MAX_PROCS]; > int groups[MAX_PROCS]; > > memset(pids, 0 , sizeof(pids)); > memset(groups, 0 , sizeof(groups)); > > again: > for (nprocs = 0, i = 1; i < argc; i++) { > int num = atoi(argv[i]); > int j; > for (j = 0; j < num; j++) { > groups[nprocs + j] = i; > } > nprocs += num; > } > > shmid = shmget(IPC_PRIVATE, SHMSIZE, IPC_CREAT | 0666); > if (shmid == -1) { > perror("shmget"); > exit(1); > } > > shared = shmat(shmid, NULL, 0); > shared_flag = (struct start_stop *)shared; > > memset(shared, 0, SHMSIZE); > shmctl(shmid, IPC_RMID, 0); > > for (i = 0; i < nprocs; i++) { > int ret; > ret = fork(); > if (ret == 0) { > worker(i); > exit(0); > } else if (ret == -1) { > perror("fork"); > exit(0); > } > pids[i] = ret; > } > sleep(1); > for (i = 0; i < nprocs; i++) > assign_to(pids[i], groups[i]); > sleep(1); > ia64_mf(); > shared_flag->go = 1; > > for (i = 0; i < nprocs; i++) { > int status; > waitpid(pids[i], &status, 0); > } > goto again; > > return 0; > } _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Fri, Dec 14, 2007 at 07:58:37PM +0900, KAMEZAWA Hiroyuki wrote: > Here is much easier test. Thanks for the test! Let me see if I can reproduce it here. -- regards, Dhaval _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 14/12/2007, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > Here is much easier test. > (I'm sorry I'll be absent tomorrow.) > > the number of cpus is 8. ia64/NUMA. > > The hang occurs when the number of tasks is not smaller than available cpus. > Can be a hint ? > > [ ... ] > > [root@rhel51GA testpro]#./batech-test.sh > no hang. > > [root@rhel51GA testpro]#taskset 0f ./batech-test.sh > no hang > > [root@rhel51GA testpro]#taskset 03 ./batech-test.sh > hang. have you tried : [root@rhel51GA testpro]#taskset 01 ./batech-test.sh hang? just to be sure SMP does matter here (most likely yes, I guess). TIA, > > Thanks, > -Kame > -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
Re: Re: Hang with fair cgroup scheduler (reproducer is attached.)
- >have you tried : > >[root@rhel51GA testpro]#taskset 01 ./batech-test.sh > yes >hang? > no. >just to be sure SMP does matter here (most likely yes, I guess). > maybe. As far as I tested, there was no hang if the number of cpus is 1. Regards, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Fri, Dec 14, 2007 at 01:47:13PM +0100, Dmitry Adamushko wrote: > On 14/12/2007, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > Here is much easier test. > > (I'm sorry I'll be absent tomorrow.) > > > > the number of cpus is 8. ia64/NUMA. > > > > The hang occurs when the number of tasks is not smaller than available cpus. > > Can be a hint ? > > > > [ ... ] > > > > [root@rhel51GA testpro]#./batech-test.sh > > no hang. > > > > [root@rhel51GA testpro]#taskset 0f ./batech-test.sh > > no hang > > > > [root@rhel51GA testpro]#taskset 03 ./batech-test.sh > > hang. > > have you tried : > > [root@rhel51GA testpro]#taskset 01 ./batech-test.sh > > hang? > > just to be sure SMP does matter here (most likely yes, I guess). > NUMA? I am not able to reproduce it here locally on an x86 8 CPU box. -- regards, Dhaval _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
>> just to be sure SMP does matter here (most likely yes, I guess). >> > >NUMA? I am not able to reproduce it here locally on an x86 8 CPU box. > yes. I used NUMA. 2 Nodes/4CPU x 2 Hmm.. Thanks, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Fri, Dec 14, 2007 at 11:24:28PM +0900, kamezawa.hiroyu@jp.fujitsu.com wrote: > >> just to be sure SMP does matter here (most likely yes, I guess). > >> > > > >NUMA? I am not able to reproduce it here locally on an x86 8 CPU box. > > > yes. I used NUMA. 2 Nodes/4CPU x 2 > OK, I got hold of an IA64 box, non numa and have managed to reproduce it. > Hmm.. > > Thanks, > -Kame -- regards, Dhaval _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 14/12/2007, Dhaval Giani <dhaval@linux.vnet.ibm.com> > > > Actually no, its another bug. Thanks for the program! > Humm... this crash is very likely to be caused by the same bug. It just reveals itself in a different place, but effectivelly the pattern looks similar. Anyway, the rb-tree gets corrupted... and for both cases, at the very least the 'current' must be within the tree. I think, if you repeat your test a number of times, you'll likely get the very same crash as was reported by Kame. ia64 does define __ARCH_WANT_UNLOCKED_CTXSW (I checked against 2.6.23.1 that I have at hand) x86 -- not (it's not reproducible there, right?) so for ia64 task_running() makes use of 'p->oncpu' to determine whether a given task is currently running (as opposed to 'rq->curr == p' otherwise)... But at first glance, it looks like there shouldn't be situations leading to some sort of de-synchronization in determining the real 'current'. Will look at it closer. > > -- > regards, > Dhaval > -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
> [ ... ] > > [<a0000001002e0480>] rb_erase+0x300/0x7e0 > [<a000000100076290>] __dequeue_entity+0x70/0xa0 > [<a000000100076300>] set_next_entity+0x40/0xa0 > [<a0000001000763a0>] set_curr_task_fair+0x40/0xa0 > [<a000000100078d90>] sched_move_task+0x2d0/0x340 > [<a000000100078e20>] cpu_cgroup_attach+0x20/0x40 > > [ ... ] argh... it's a consequence of the 'current is not kept within the tree" indeed. When sched_move_task() is called for the 'current' (running on another CPU), we get the following: ... running = task_running(rq, tsk); on_rq = tsk->se.on_rq; if (on_rq) { dequeue_task(rq, tsk, 0); if (unlikely(running)) tsk->sched_class->put_prev_task(rq, tsk); } [1] tsk->sched_class->put_prev_task() actually _inserts_ 'tsk' back into the cfs_rq of its _old_ group : set_task_cfs_rq(tsk, task_cpu(tsk)); [2] now task.se->cfs_rq gets changed if (on_rq) { if (unlikely(running)) tsk->sched_class->set_curr_task(rq); [3] and now, tsk->sched_class->set_curr_task(rq) _removes_ the 'current' from the tree... but this tree belongs to the _new_ group (the task is still within the 'old_group->cfs_rq->rb_tree') ---> oops! enqueue_task(rq, tsk, 0); } Anyway, I have to admit that this problem is a consequence of the special-case treatment for the 'current' by 'dequeue/enqueue_task()'... it makes the interface less transparent indeed. /me thinking on how to get it fixed (e.g. set_task_cfs_rq() might take care of it) or just get this special-case issue removed (have to check whether we lose anything in this case)... sigh. -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Fri, 14 Dec 2007, Dmitry Adamushko wrote: > > argh... it's a consequence of the 'current is not kept within the tree" indeed. > Thanks Dmitry for tracking this down. Although I'm still not convinced we hit the same bug. But I'm going to go ahead and release 2.6.24-rc5-rt1 anyway. When you have a fix, please CC me and I'll add it to -rt2. Note: I've added a bunch of logdev (see http://rostedt.homelinux.com/logdev/README) and I kicked off the hackbench again. I'll let it run overnight, and if it hits the bug, it will give me a lot more output to let me know what actually happened. Thanks, -- Steve _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 14/12/2007, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 14 Dec 2007, Dmitry Adamushko wrote: > > > > > argh... it's a consequence of the 'current is not kept within the tree" indeed. > > > > Thanks Dmitry for tracking this down. My analysis was flawed (hmm... me was under control of Belgium beer :-) The task in not on the runqueue (p->on_rq == 0) at the moment when put_prev_task_fair() and set_curr_task_fair() get its turn in sched_move_task()... so dequeue/enqueue_entity() are not triggered, that's good. so back to the square #0. > Thanks, > > -- Steve -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Sat, Dec 15, 2007 at 11:22:08AM +0100, Dmitry Adamushko wrote: > On 14/12/2007, Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Fri, 14 Dec 2007, Dmitry Adamushko wrote: > > > > > > > > argh... it's a consequence of the 'current is not kept within the tree" indeed. > > > > > > > Thanks Dmitry for tracking this down. > > My analysis was flawed (hmm... me was under control of Belgium beer :-) > > The task in not on the runqueue (p->on_rq == 0) at the moment when > put_prev_task_fair() and set_curr_task_fair() get its turn in > sched_move_task()... so dequeue/enqueue_entity() are not triggered, > that's good. > Again, I am probably missing something, but if on_rq == 0, then how is set_curr_task_fair() getting called? -- regards, Dhaval _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 15/12/2007, Dhaval Giani <dhaval@linux.vnet.ibm.com> wrote: > On Sat, Dec 15, 2007 at 11:22:08AM +0100, Dmitry Adamushko wrote: > > On 14/12/2007, Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > On Fri, 14 Dec 2007, Dmitry Adamushko wrote: > > > > > > > > > > > argh... it's a consequence of the 'current is not kept within the tree" indeed. > > > > > > > > > > Thanks Dmitry for tracking this down. > > > > My analysis was flawed (hmm... me was under control of Belgium beer :-) > > > > The task in not on the runqueue (p->on_rq == 0) at the moment when > > put_prev_task_fair() and set_curr_task_fair() get its turn in > > sched_move_task()... so dequeue/enqueue_entity() are not triggered, > > that's good. > > > > Again, I am probably missing something, but if on_rq == 0, then how is > set_curr_task_fair() getting called? > ... running = task_running(rq, tsk); on_rq = tsk->se.on_rq; // let's say on_rq == 1 , i.e. the task is on the runqueue if (on_rq) { dequeue_task(rq, tsk, 0); // now tsk->se.on_rq becomes 0 if (unlikely(running)) tsk->sched_class->put_prev_task(rq, tsk); // put_prev_task() --> put_prev_entity() checks for 'tsk->se.on_rq' to determine whether __enqueue_entity() must be done ---> and it's 0 in our case. [ it can be non-zero for the following path : schedule() --> put_prev_task(..., prev) when deactivate_task(..., prev) was not previously called in schedule(), i.e. 'prev' was preempted ] tsk->se.on_rq will become 1 only after enqueue_task(). As a result, tsk->se.on_rq is still 0 when set_curr_task() is executed. does it make sense now? > -- > regards, > Dhaval > -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 15/12/2007, Dmitry Adamushko <dmitry.adamushko@gmail.com> wrote: > > My analysis was flawed (hmm... me was under control of Belgium beer :-) > ok, I've got another one (just in case... well, this late hour to be blamed now :-/) according to Dhaval, we have a crash on ia64 (it's also the arch for the original report) and it's not reproducible on an otherwise similar (wrt. # of cpus) x86. (1) The difference that comes first in mind is that ia64 makes use of __ARCH_WANT_UNLOCKED_CTXSW dimm@earth:~/storage/kernel/linux-2.6$ grep -rn __ARCH_WANT_UNLOCKED_CTXSW include/ include/linux/sched.h:947:#ifdef __ARCH_WANT_UNLOCKED_CTXSW include/asm-mips/system.h:216:#define __ARCH_WANT_UNLOCKED_CTXSW include/asm-ia64/system.h:259:#define __ARCH_WANT_UNLOCKED_CTXSW (2) now, in this case (and for SMP) task_running() effectively becomes { return p->oncpu; } (3) consider a case of the context switch between prev --> next on CPU #0 'next' has preempted 'prev' (4) context_swicth() : next->oncpu becomes '1' as the result of: [1] context_switch() --> prepare_task_switch() --> prepare_lock_switch(next) --> next->oncpu = 1 prev->oncpu becomes '0' as the result of: [2] context_switch() --> finish_task_switch() --> finish_lock_switch(prev) --> prev->oncpu = 0 [1] takes place at the very _beginning_ of context_switch() _and_ one more thing is that rq->lock gets unlocked. [2] takes place at the very _end_ of context_switch() Now recall what's task_running() in our case ( it's "return task->oncpu" ) As a result, between [1] and [2] we have 2 tasks on a single CPU for which task_running() will return '1' and their runqueue is _unlocked_. (5) now consider sched_move_task() running on another CPU #1. due to 'UNLOCKED_CTXSW' it can successfully lock the rq of CPU #0 let's say it's called for 'prev' task (the one being scheduled out on CPU #0 at this very moment) as we remember, task_running() returns '1' for it (CPU #0 haven't reached yet point [2] as described in (4) above) 'prev' is currently on the runqueue (prev->se.on_rq == 1) and within the tree. what happens is as follows: - dequeue_task() removes it from the tree ; - put_prev_task() makes cfs_rq->curr = NULL ; se == prev.se here... so e.g. __enqueue_entity() is not called for 'prev' - set_curr_task() --> set_curr_task_fair() and here things become interesting. static void set_curr_task_fair(struct rq *rq) { struct sched_entity *se = &rq->curr->se; for_each_sched_entity(se) set_next_entity(cfs_rq_of(se), se); } so 'se' actually belongs to the 'next' on CPU #0 next->on_rq == 1 (obviously, as dequeue_task() in sched_move_task() was done for 'prev' !) and now, set_next_entity() does __dequeue_entity() for 'next' which is _not_ within the tree !!! (it's the real 'current' on CPU #0) that's why the reported oops: > [<a0000001002e0480>] rb_erase+0x300/0x7e0 > [<a000000100076290>] __dequeue_entity+0x70/0xa0 > [<a000000100076300>] set_next_entity+0x40/0xa0 > [<a0000001000763a0>] set_curr_task_fair+0x40/0xa0 > [<a000000100078d90>] sched_move_task+0x2d0/0x340 > [<a000000100078e20>] cpu_cgroup_attach+0x20/0x40 or maybe there is also a possibility of the rb-tree being corrupted as a result and having a crash somewhere later (the original report had another backtrace) hum... does this analysis make sense to somebody else now? -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
Dhaval, so following the analysis in the previous mail... here is a test patch. Could you please give it a try? TIA, (enclosed non white-space broken version) --- --- a/kernel/sched.c +++ b/kernel/sched.c @@ -7360,7 +7360,7 @@ void sched_move_task(struct task_struct *tsk) update_rq_clock(rq); - running = task_running(rq, tsk); + running = (rq->curr == tsk); on_rq = tsk->se.on_rq; if (on_rq) { --- -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Sun, Dec 16, 2007 at 01:00:07AM +0100, Dmitry Adamushko wrote: > Dhaval, > > so following the analysis in the previous mail... here is a test > patch. Could you please give it a try? > Yep, it works! Tested-by: Dhaval Giani <dhaval@linux.vnet.ibm.com> thanks, -- regards, Dhaval _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
* Dmitry Adamushko <dmitry.adamushko@gmail.com> wrote: > --- a/kernel/sched.c > +++ b/kernel/sched.c > @@ -7360,7 +7360,7 @@ void sched_move_task(struct task_struct *tsk) > > update_rq_clock(rq); > > - running = task_running(rq, tsk); > + running = (rq->curr == tsk); > on_rq = tsk->se.on_rq; thanks, i've queued this up (pending more testing). Btw., you should be able to force the ia64 scheduling by adding this to the very top of include/linux/sched.h: #define __ARCH_WANT_UNLOCKED_CTXSW #define __ARCH_WANT_INTERRUPTS_ON_CTXSW Ingo _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 16/12/2007, Ingo Molnar <mingo@elte.hu> wrote: > > * Dmitry Adamushko <dmitry.adamushko@gmail.com> wrote: > > > --- a/kernel/sched.c > > +++ b/kernel/sched.c > > @@ -7360,7 +7360,7 @@ void sched_move_task(struct task_struct *tsk) > > > > update_rq_clock(rq); > > > > - running = task_running(rq, tsk); > > + running = (rq->curr == tsk); > > on_rq = tsk->se.on_rq; > > thanks, i've queued this up (pending more testing). btw., sched_setscheduler() and rt_mutex_setprio() are also affected (in general, anything that may call put_prev_task/set_curr_task() relying task_running()). Will see, maybe we may come up with smth better than just replacing task_running() with (rq->curr == tsk) there. > Btw., you should be able to force the ia64 scheduling by adding this to > the very top of include/linux/sched.h: > > #define __ARCH_WANT_UNLOCKED_CTXSW > #define __ARCH_WANT_INTERRUPTS_ON_CTXSW Yeah, with both we even get ARM behavior. Can be a good test indeed. > > Ingo > -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
Ingo, what about the following patch instead? maybe task_is_current() would be a better name though. Steven, I guess, there is some analogue of UNLOCKED_CTXSW on -rt (to reduce contention for rq->lock). So there can be a race schedule() vs. rt_mutex_setprio() or sched_setscheduler() for some paths that might explain crashes you have been observing? I haven't analyzed this case for -rt, so I'm just throwing in the idea in case it can be useful. --------------------------------------------------- From: Dmitry Adamushko <dmitry.adamushko@gmail.com> sched: introduce task_current() Some services (e.g. sched_setscheduler(), rt_mutex_setprio() and sched_move_task()) must handle a given task differently in case it's the 'rq->curr' task on its run-queue. The task_running() interface is not suitable for determining such tasks for platforms with one of the following options: #define __ARCH_WANT_UNLOCKED_CTXSW #define __ARCH_WANT_INTERRUPTS_ON_CTXSW Due to the fact that it makes use of 'p->oncpu == 1' as a criterion but such a task is not necessarily 'rq->curr'. The detailed explanation is available here: https://lists.linux-foundation.org/pipermail/containers/2007-December/009262.html Signed-off-by: Dmitry Adamushko <dmitry.adamushko@gmail.com> --- diff --git a/kernel/sched.c b/kernel/sched.c index dc6fb24..15d088b 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -619,10 +619,15 @@ EXPORT_SYMBOL_GPL(cpu_clock); # define finish_arch_switch(prev) do { } while (0) #endif +static inline int task_current(struct rq *rq, struct task_struct *p) +{ + return rq->curr == p; +} + #ifndef __ARCH_WANT_UNLOCKED_CTXSW static inline int task_running(struct rq *rq, struct task_struct *p) { - return rq->curr == p; + return task_current(rq, p); } static inline void prepare_lock_switch(struct rq *rq, struct task_struct *next) @@ -651,7 +656,7 @@ static inline int task_running(struct rq *rq, struct task_struct *p) #ifdef CONFIG_SMP return p->oncpu; #else - return rq->curr == p; + return task_current(rq, p); #endif } @@ -3340,7 +3345,7 @@ unsigned long long task_sched_runtime(struct task_struct *p) rq = task_rq_lock(p, &flags); ns = p->se.sum_exec_runtime; - if (rq->curr == p) { + if (task_current(rq, p)) { update_rq_clock(rq); delta_exec = rq->clock - p->se.exec_start; if ((s64)delta_exec > 0) @@ -4033,7 +4038,7 @@ void rt_mutex_setprio(struct task_struct *p, int prio) oldprio = p->prio; on_rq = p->se.on_rq; - running = task_running(rq, p); + running = task_current(rq, p); if (on_rq) { dequeue_task(rq, p, 0); if (running) @@ -4334,7 +4339,7 @@ recheck: } update_rq_clock(rq); on_rq = p->se.on_rq; - running = task_running(rq, p); + running = task_current(rq, p); if (on_rq) { deactivate_task(rq, p, 0); if (running) @@ -7360,7 +7365,7 @@ void sched_move_task(struct task_struct *tsk) update_rq_clock(rq); - running = task_running(rq, tsk); + running = task_current(rq, tsk); on_rq = tsk->se.on_rq; if (on_rq) { --- _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Sun, 16 Dec 2007, Dmitry Adamushko wrote: > Steven, > > I guess, there is some analogue of UNLOCKED_CTXSW on -rt > (to reduce contention for rq->lock). > So there can be a race schedule() vs. rt_mutex_setprio() or sched_setscheduler() > for some paths that might explain crashes you have been observing? > > I haven't analyzed this case for -rt, so I'm just throwing in the idea in case it can be useful. Dmitry, Thanks! I've been watching this thread on the sidelines with keen interest. I ran hackbench with my logging over the weekend, and after some 30 hours of running, it finally crashed and gave me a dump of a lot of interactions in the put_prev_enitity and pick_next_task. I'll analyze this more on Monday. -- Steve _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Sun, 16 Dec 2007, Dmitry Adamushko wrote: > Steven, > > I guess, there is some analogue of UNLOCKED_CTXSW on -rt > (to reduce contention for rq->lock). > So there can be a race schedule() vs. rt_mutex_setprio() or sched_setscheduler() > for some paths that might explain crashes you have been observing? > > I haven't analyzed this case for -rt, so I'm just throwing in the idea in case it can be useful. I haven't fully analyzed this either, but will look much deeper tomorrow. I wanted to show you this first just to see if you can easily spot what went wrong. I used my logdev logging device http://rostedt.homelinux.com/logdev The patch that I used to add the logging is here: http://rostedt.homelinux.com/rt-bug/debug-logdev.patch To understand this. lfcnprint(...) is just like printk, but it will print output the following format: [<timestamp>] cpu:<cpu#> (<current-comm>:<current-pid>) <function>:<line#> <printk-fmt> The tags of lmark() is just [<timestamp>] cpu:<cpu#> (<current-comm>:<current-pid>) <function>:<line#> On context switches, we get >>>> IN LOGDEV SWITCH <<<< cpu: <cpu#> CPU=<cpu#> [<timestamp>] <prev>:<prev-pid>(<prios>:<task-state>) -->> <next>:<next-pid>(<prios>) The full dmesg with logdump and error backtrace is here: http://rostedt.homelinux.com/rt-bug/rt-bug.log Here's a little snippet of where things went wrong. [94359.652019] cpu:3 (hackbench:1658) pick_next_task_fair:1036 nr_running=1 [94359.652020] cpu:3 (hackbench:1658) pick_next_entity:625 se=ffff810009020800 [94359.652021] cpu:0 (hackbench:1473) put_prev_entity:631 [94359.652022] cpu:3 (hackbench:1658) pick_next_entity:625 se=ffff81003906b5a8 [94359.652022] cpu:2 (softirq-timer/2:32) put_prev_task_rt:283 [94359.652023] cpu:0 (hackbench:1473) put_prev_entity:631 >>>> IN LOGDEV SWITCH <<<< cpu:3 CPU=3 [94359.652023] hackbench:1658(120:120:120:D) -->> hackbench:1586(120:120:120) >>>> IN LOGDEV SWITCH <<<< cpu:2 CPU=2 [94359.652024] softirq-timer/2:32(49:115:49:D) -->> softirq-rcu/2:39(49:115:49) >>>> IN LOGDEV SWITCH <<<< cpu:0 CPU=0 [94359.652025] hackbench:1473(120:120:120:R) -->> hackbench:1591(49:120:120) [94359.652029] cpu:3 (hackbench:1586) put_prev_entity:631 [94359.652030] cpu:2 (softirq-rcu/2:39) move_tasks:2472 [94359.652030] cpu:3 (hackbench:1586) put_prev_entity:631 [94359.652032] cpu:3 (hackbench:1586) pick_next_task_fair:1036 nr_running=1 [94359.652033] cpu:3 (hackbench:1586) pick_next_entity:625 se=ffff810009020800 [94359.652034] cpu:3 (hackbench:1586) pick_next_entity:625 se=ffff810014c7ab18 [94359.652034] cpu:2 (softirq-rcu/2:39) put_prev_task_rt:283 >>>> IN LOGDEV SWITCH <<<< cpu:3 CPU=3 [94359.652035] hackbench:1586(120:120:120:T) -->> hackbench:1623(120:120:120) [94359.652036] cpu:2 (softirq-rcu/2:39) pick_next_task_fair:1036 nr_running=1 [94359.652038] cpu:2 (softirq-rcu/2:39) pick_next_entity:625 se=0000000000000000 I see that softirq-rcu on cpu 2 started doing a move_tasks, when it got to the state where nr_running returned 1 and the se from pick_next_entity was NULL. This was the run on 2.6.24-rc5-rt1. I'll look deeper into this on Monday, but if something jumps out at you, please let me know. Thanks, -- Steve _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Sun, 16 Dec 2007 09:58:21 +0530 Dhaval Giani <dhaval@linux.vnet.ibm.com> wrote: > On Sun, Dec 16, 2007 at 01:00:07AM +0100, Dmitry Adamushko wrote: > > Dhaval, > > > > so following the analysis in the previous mail... here is a test > > patch. Could you please give it a try? > > > > Yep, it works! > > Tested-by: Dhaval Giani <dhaval@linux.vnet.ibm.com> > Works for me, too !! Thanks, -Kame _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On 17/12/2007, Steven Rostedt <rostedt@goodmis.org> wrote: > > Here's a little snippet of where things went wrong. > > [94359.652019] cpu:3 (hackbench:1658) pick_next_task_fair:1036 nr_running=1 > [94359.652020] cpu:3 (hackbench:1658) pick_next_entity:625 se=ffff810009020800 > [94359.652021] cpu:0 (hackbench:1473) put_prev_entity:631 > [94359.652022] cpu:3 (hackbench:1658) pick_next_entity:625 se=ffff81003906b5a8 > [94359.652022] cpu:2 (softirq-timer/2:32) put_prev_task_rt:283 > [94359.652023] cpu:0 (hackbench:1473) put_prev_entity:631 > >>>> IN LOGDEV SWITCH <<<< cpu:3 > CPU=3 [94359.652023] hackbench:1658(120:120:120:D) -->> hackbench:1586(120:120:120) > >>>> IN LOGDEV SWITCH <<<< cpu:2 > CPU=2 [94359.652024] softirq-timer/2:32(49:115:49:D) -->> softirq-rcu/2:39(49:115:49) > >>>> IN LOGDEV SWITCH <<<< cpu:0 > CPU=0 [94359.652025] hackbench:1473(120:120:120:R) -->> hackbench:1591(49:120:120) > [94359.652029] cpu:3 (hackbench:1586) put_prev_entity:631 > [94359.652030] cpu:2 (softirq-rcu/2:39) move_tasks:2472 > [94359.652030] cpu:3 (hackbench:1586) put_prev_entity:631 > [94359.652032] cpu:3 (hackbench:1586) pick_next_task_fair:1036 nr_running=1 > [94359.652033] cpu:3 (hackbench:1586) pick_next_entity:625 se=ffff810009020800 > [94359.652034] cpu:3 (hackbench:1586) pick_next_entity:625 se=ffff810014c7ab18 > [94359.652034] cpu:2 (softirq-rcu/2:39) put_prev_task_rt:283 > >>>> IN LOGDEV SWITCH <<<< cpu:3 > CPU=3 [94359.652035] hackbench:1586(120:120:120:T) -->> hackbench:1623(120:120:120) > [94359.652036] cpu:2 (softirq-rcu/2:39) pick_next_task_fair:1036 nr_running=1 > [94359.652038] cpu:2 (softirq-rcu/2:39) pick_next_entity:625 se=0000000000000000 > > I see that softirq-rcu on cpu 2 started doing a move_tasks, when it got to > the state where nr_running returned 1 and the se from pick_next_entity was > NULL. move_task() is likely to be run from schedule() --> idle_balance() , for our case it means that 'softirq-rcu' (which is a RT task) went to sleep and it was the last task on this CPU (rq->nr_running == 0 --> idle_balance() was triggered). It may be related, maybe not. One 'abnormal' thing (at least, it occurs only once in this log. Should be checked wheather it happens when the system works fine) is that a few iterations before the oops happens we observe the following pattern: CPU=2 [94359.651930] hackbench:1932(120:120:120:T) -->> hackbench:1591(120:120:120) CPU=2 [94359.651980] hackbench:1591(49:120:120:T) -->> swapper:0(140:120:140) swapper (idle) --> softirq-timer (RT) softirq-timer (RT) --> softirq-rcu (RT) softirq-rcu(RT) --> picks up se == 0 for SCHED_NORMAL upon scheduling out ---> OOPS 'hackbench' was of SCHED_NORMAL upon scheduling _in_, and it's of RT type (prio: 49 and schedule() --> put_prev_task_rt()) upon scheduling _out_. Unless you run some modified version of 'hackbench', it doesn't chenge scheduling classes... so maybe a lifted prio is a consequence of the resource contention with some RT task ? This 'hackbench' was the last SCHED_NORMAL task to run on this CPU... so however this NORMAL -> RT transition happened, it might leave a sched_fair's runqueue corrupted... (Will try to look more when time allows). > > Thanks, > > -- Steve -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > > so following the analysis in the previous mail... here is a test > > > patch. Could you please give it a try? > > > > > > > Yep, it works! > > > > Tested-by: Dhaval Giani <dhaval@linux.vnet.ibm.com> > > > Works for me, too !! thanks guys, i'll push Dmitry's fix out with the next scheduler git push. Ingo _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
On Mon, 17 Dec 2007, Dmitry Adamushko wrote: > > It may be related, maybe not. One 'abnormal' thing (at least, it > occurs only once in this log. Should be checked wheather it happens > when the system works fine) is that a few iterations before the oops > happens we observe the following pattern: > > CPU=2 [94359.651930] hackbench:1932(120:120:120:T) -->> > hackbench:1591(120:120:120) > > CPU=2 [94359.651980] hackbench:1591(49:120:120:T) -->> swapper:0(140:120:140) Note: the 'T' should be a 'D' because my logdev didn't add the change that -rt does (adding a 'M' state). Thanks for noticing. The -rt patch has more priority inheritance situations than vanilla kernel (sleeping spinlocks or semaphors, and even the Preempt RCU Boost logic). > > swapper (idle) --> softirq-timer (RT) > softirq-timer (RT) --> softirq-rcu (RT) > softirq-rcu(RT) --> picks up se == 0 for SCHED_NORMAL upon scheduling > out ---> OOPS > > 'hackbench' was of SCHED_NORMAL upon scheduling _in_, and it's of RT > type (prio: 49 and schedule() --> put_prev_task_rt()) upon scheduling > _out_. > > Unless you run some modified version of 'hackbench', it doesn't chenge > scheduling classes... so maybe a lifted prio is a consequence of the > resource contention with some RT task ? Yes. Which means it could be an spinlock, mutex, semaphore or RCU read lock. But since it is in the TASK_UNINTERRUPTIBLE state, I'm willing to bet this is a mutex (or converted spinlock). > > This 'hackbench' was the last SCHED_NORMAL task to run on this CPU... > so however this NORMAL -> RT transition happened, it might leave a > sched_fair's runqueue corrupted... Could very well have. The PI uses task_setprio (aka. rt_mutex_setprio) to raise the priority. I'll start looking there. > > (Will try to look more when time allows). Thanks, I'll probably spend the rest of the day on this. -- Steve _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
[ trimmed the cc' list ] On 17/12/2007, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 17 Dec 2007, Dmitry Adamushko wrote: > > > > > It may be related, maybe not. One 'abnormal' thing (at least, it > > occurs only once in this log. Should be checked wheather it happens > > when the system works fine) is that a few iterations before the oops > > happens we observe the following pattern: > > > > CPU=2 [94359.651930] hackbench:1932(120:120:120:T) -->> > > hackbench:1591(120:120:120) > > > > CPU=2 [94359.651980] hackbench:1591(49:120:120:T) -->> swapper:0(140:120:140) > > Thanks for noticing. The -rt patch has more priority inheritance > situations than vanilla kernel (sleeping spinlocks or semaphors, and even > the Preempt RCU Boost logic). One more thing is that we don't actually see a point where that 'hackbench' gets its priority lifted. It was scheduled in as a NORMAL task and scheduled out as a RT one. i.e. the task got its prio elevated while it was running... a contention with the task on another CPU? anyway, i.e. this task must have 'p->se.on_rq == 1' and I'd expect to see "switched_to_rt" message somewhere in between... hmm? (check_class_changed() shoud have been called in task_setprio()). btw., we do see one 'switched_from_rt --> switched_to_fair' case for another 'hackbench' on CPU #0... according to traces, this one might get a prio lifted while sleeping (it got scheduled in as a RT task). > > -- Steve > -- Best regards, Dmitry Adamushko _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers