2.6.33.[56]-rt23: howto create repeatable explosion in wakeup_next_waiter()
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc > 4.3, else testcases will segfault due to a gcc bug.
2. apply patchlet so you can run testcases/realtime/perf/latency/run.sh at all.
pthread_cond_many.c renamed from pthread_cond_many.c.org +1 -1
@@ -259,7 +259,7 @@ void usage(void)int parse_args (int c,char *v) { -int handled; +int handled =1 ;switch (c) {case 'h' :usage ();
3. add --realtime for no particular reason.
run.sh renamed from run.sh.org +2 -2
@@ -22,7 +22,7 @@ make # process to run realtime. The remainder of the processes (if any) # will run non-realtime in any case. -nthread=5000 +nthread=500 iter=400 nproc=5
@@ -39,7 +39,7 @@ i=0 i=1 while test $i -lt $nproc do - ./pthread_cond_many --broadcast -i $iter -n $nthread > $nthread.$iter.$nproc.$i.out & + ./pthread_cond_many --realtime --broadcast -i $iter -n $nthread > $nthread.$iter.$nproc.$i.out & i=`expr $i + 1` done wait
4. run it.
What happens here is we hit WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comes WARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.
With explosion avoidance, I also see pendowner->pi_blocked_on->task == NULL at times, but that, as !pendowner->pi_blocked_on, seems to be fallout. The start of bad juju is always pi_blocked_on != waiter.
[ 141.609268] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 141.609268] IP: [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] PGD 20e174067 PUD 20e253067 PMD 0 [ 141.609268] Oops: 0000 [#1] PREEMPT SMP [ 141.609268] last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map [ 141.609268] CPU 0 [ 141.609268] Pid: 8154, comm: pthread_cond_ma Tainted: G W 2.6.33.6-rt23 #12 MS-7502/MS-7502 [ 141.609268] RIP: 0010:[<ffffffff8106856d>] [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP: 0018:ffff88020e3cdd78 EFLAGS: 00010097 [ 141.609268] RAX: 0000000000000000 RBX: ffff8801e8eba5c0 RCX: 0000000000000000 [ 141.609268] RDX: ffff880028200000 RSI: 0000000000000046 RDI: 0000000000000009 [ 141.609268] RBP: ffff88020e3cdda8 R08: 0000000000000002 R09: 0000000000000000 [ 141.609268] R10: 0000000000000005 R11: 0000000000000000 R12: ffffffff81659068 [ 141.609268] R13: ffff8801e8ebdb58 R14: 0000000000000000 R15: ffff8801e8ebac08 [ 141.609268] FS: 00007f664d539700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 [ 141.609268] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.609268] CR2: 0000000000000058 CR3: 0000000214266000 CR4: 00000000000006f0 [ 141.609268] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.609268] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 141.609268] Process pthread_cond_ma (pid: 8154, threadinfo ffff88020e3cc000, task ffff88020e2a4700) [ 141.609268] Stack: [ 141.609268] 0000000000000000 ffffffff81659068 0000000000000202 0000000000000000 [ 141.609268] <0> 0000000000000000 0000000080001fda ffff88020e3cddc8 ffffffff812fec48 [ 141.609268] <0> ffffffff81659068 0000000000606300 ffff88020e3cddd8 ffffffff812ff1b9 [ 141.609268] Call Trace: [ 141.609268] [<ffffffff812fec48>] rt_spin_lock_slowunlock+0x43/0x61 [ 141.609268] [<ffffffff812ff1b9>] rt_spin_unlock+0x46/0x48 [ 141.609268] [<ffffffff81067d7f>] do_futex+0x83c/0x935 [ 141.609268] [<ffffffff810c26ce>] ? handle_mm_fault+0x6de/0x6f1 [ 141.609268] [<ffffffff81067e36>] ? do_futex+0x8f3/0x935 [ 141.609268] [<ffffffff81067fba>] sys_futex+0x142/0x154 [ 141.609268] [<ffffffff81020eb0>] ? do_page_fault+0x23e/0x28e [ 141.609268] [<ffffffff81004aa7>] ? math_state_restore+0x3d/0x3f [ 141.609268] [<ffffffff81004b08>] ? do_device_not_available+0xe/0x12 [ 141.609268] [<ffffffff81002c5b>] system_call_fastpath+0x16/0x1b [ 141.609268] Code: c7 09 6d 41 81 e8 ac 34 fd ff 4c 39 ab 70 06 00 00 74 11 be 47 02 00 00 48 c7 c7 09 6d 41 81 e8 92 34 fd ff 48 8b 83 70 06 00 00 <4c> 39 60 58 74 11 be 48 02 00 00 48 c7 c7 09 6d 41 81 e8 74 34 [ 141.609268] RIP [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP <ffff88020e3cdd78> [ 141.609268] CR2: 0000000000000058 [ 141.609268] ---[ end trace 58805b944e6f93ce ]--- [ 141.609268] note: pthread_cond_ma[8154] exited with preempt_count 2
(5. eyeball locks.. -> zzzzt -> report -> eyeball..)
-Mike
FWIW, 2.6.31.14-rt21 explodes too (darn).
Quoted 2 lines · Mike Galbraith
FWIW, 2.6.31.14-rt21 explodes too (darn).
What about .29-rt ?
Thanks,
tglx
(switches back to primary address.. dang evolution)
Quoted 6 lines · Thomas Gleixner
Quoted 2 lines · Mike Galbraith
FWIW, 2.6.31.14-rt21 explodes too (darn).
What about .29-rt ?
Goes boom too.
-Mike
Cc'ing Darren.
Quoted 104 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc > 4.3, else testcases will segfault due to a gcc bug.
2. apply patchlet so you can run
testcases/realtime/perf/latency/run.shat all.pthread_cond_many.c renamed from pthread_cond_many.c.org +1
-1@@ -259,7 +259,7 @@ void usage(void)int parse_args (int c,char *v) { -int handled; +int handled =1 ;switch (c) {case 'h' :usage ();3. add --realtime for no particular reason.
run.sh renamed from run.sh.org +2
-2@@ -22,7 +22,7 @@ make # process to run realtime. The remainder of the processes (if any) # will run non-realtime in any case. -nthread=5000 +nthread=500 iter=400 nproc=5@@ -39,7 +39,7 @@ i=0 i=1 while test $i -lt $nproc do - ./pthread_cond_many --broadcast -i $iter -n $nthread > $nthread.$iter.$nproc.$i.out & + ./pthread_cond_many --realtime --broadcast -i $iter -n $nthread > $nthread.$iter.$nproc.$i.out & i=`expr $i + 1` done wait4. run it.
What happens here is we hit
WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comesWARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.With explosion avoidance, I also see
pendowner->pi_blocked_on->task== NULL at times, but that, as !pendowner->pi_blocked_on, seems to be fallout. The start of bad juju is alwayspi_blocked_on!= waiter.[ 141.609268] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 141.609268] IP: [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] PGD 20e174067 PUD 20e253067 PMD 0 [ 141.609268] Oops: 0000 [#1] PREEMPT SMP [ 141.609268] last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map [ 141.609268] CPU 0 [ 141.609268] Pid: 8154, comm: pthread_cond_ma Tainted: G W 2.6.33.6-rt23 #12 MS-7502/MS-7502 [ 141.609268] RIP: 0010:[<ffffffff8106856d>] [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP: 0018:ffff88020e3cdd78 EFLAGS: 00010097 [ 141.609268] RAX: 0000000000000000 RBX: ffff8801e8eba5c0 RCX: 0000000000000000 [ 141.609268] RDX: ffff880028200000 RSI: 0000000000000046 RDI: 0000000000000009 [ 141.609268] RBP: ffff88020e3cdda8 R08: 0000000000000002 R09: 0000000000000000 [ 141.609268] R10: 0000000000000005 R11: 0000000000000000 R12: ffffffff81659068 [ 141.609268] R13: ffff8801e8ebdb58 R14: 0000000000000000 R15: ffff8801e8ebac08 [ 141.609268] FS: 00007f664d539700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 [ 141.609268] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.609268] CR2: 0000000000000058 CR3: 0000000214266000 CR4: 00000000000006f0 [ 141.609268] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.609268] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 141.609268] Process pthread_cond_ma (pid: 8154, threadinfo ffff88020e3cc000, task ffff88020e2a4700) [ 141.609268] Stack: [ 141.609268] 0000000000000000 ffffffff81659068 0000000000000202 0000000000000000 [ 141.609268] <0> 0000000000000000 0000000080001fda ffff88020e3cddc8 ffffffff812fec48 [ 141.609268] <0> ffffffff81659068 0000000000606300 ffff88020e3cddd8 ffffffff812ff1b9 [ 141.609268] Call Trace: [ 141.609268] [<ffffffff812fec48>] rt_spin_lock_slowunlock+0x43/0x61 [ 141.609268] [<ffffffff812ff1b9>] rt_spin_unlock+0x46/0x48 [ 141.609268] [<ffffffff81067d7f>] do_futex+0x83c/0x935 [ 141.609268] [<ffffffff810c26ce>] ? handle_mm_fault+0x6de/0x6f1 [ 141.609268] [<ffffffff81067e36>] ? do_futex+0x8f3/0x935 [ 141.609268] [<ffffffff81067fba>] sys_futex+0x142/0x154 [ 141.609268] [<ffffffff81020eb0>] ? do_page_fault+0x23e/0x28e [ 141.609268] [<ffffffff81004aa7>] ? math_state_restore+0x3d/0x3f [ 141.609268] [<ffffffff81004b08>] ? do_device_not_available+0xe/0x12 [ 141.609268] [<ffffffff81002c5b>] system_call_fastpath+0x16/0x1b [ 141.609268] Code: c7 09 6d 41 81 e8 ac 34 fd ff 4c 39 ab 70 06 00 00 74 11 be 47 02 00 00 48 c7 c7 09 6d 41 81 e8 92 34 fd ff 48 8b 83 70 06 00 00 <4c> 39 60 58 74 11 be 48 02 00 00 48 c7 c7 09 6d 41 81 e8 74 34 [ 141.609268] RIP [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP <ffff88020e3cdd78> [ 141.609268] CR2: 0000000000000058 [ 141.609268] ---[ end trace 58805b944e6f93ce ]--- [ 141.609268] note: pthread_cond_ma[8154] exited with preempt_count 2(5. eyeball locks.. -> zzzzt -> report -> eyeball..)
-Mike
Quoted 4 lines · Thomas Gleixner
Cc'ing Darren.
On Wed, 7 Jul 2010, Mike Galbraith wrote:
Hi Mike,
Quoted 8 lines
Quoted 8 lines
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc> 4.3, else testcases will segfault due to a gcc bug.
Interesting, I had not hit any gcc specific issues with this. Can you point me to the bug?
Quoted 18 lines
Quoted 18 lines
2. apply patchlet so you can run
testcases/realtime/perf/latency/run.shat all.pthread_cond_many.c renamed from pthread_cond_many.c.org +1
-1@@ -259,7 +259,7 @@ void usage(void)int parse_args (int c,char *v) { -int handled; +int handled =1 ;switch (c) {case 'h' :usage ();3. add --realtime for no particular reason.
Quoted 8 lines
run.sh make · line 25
Quoted 8 lines
run.sh renamed from run.sh.org +1
-1@@ -22,7 +22,7 @@ make # process to run realtime. The remainder of the processes (if any) # will run non-realtime in any case. -nthread=5000 +nthread=500
Was this just to lighten the load, or was it required to reproduce?
Quoted 13 lines
run.sh i=0 · line 45
Quoted 13 lines
run.sh renamed from run.sh.org +1
-1iter=400 nproc=5@@ -39,7 +39,7 @@ i=0 i=1 while test $i -lt $nproc do - ./pthread_cond_many --broadcast -i $iter -n $nthread> $nthread.$iter.$nproc.$i.out& + ./pthread_cond_many --realtime --broadcast -i $iter -n $nthread> $nthread.$iter.$nproc.$i.out& i=`expr $i + 1` done wait
We'll do an audit and see if any pthread_cond_many patches have been dropped, or just fix the above issues if not.
Quoted 2 lines
Quoted 2 lines
4. run it.
Which architecture?
Glibc version?
I see kernel version is: 2.6.33.6-rt23, have you reproduced this on earlier kernel versions as well? Any 2.6.31 rt kernel would be a good data point.
Is this immediately reproducible for you?
I see a possibly fault occuring in the stack, if you run with mlockall(), does the problem go away? (assuming not, but an easy thing to test).
Nothing comes to mind re. cause quite yet, will have to dig into it.
--
Darren
Quoted 55 lines
Quoted 55 lines
What happens here is we hit
WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comesWARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.With explosion avoidance, I also see
pendowner->pi_blocked_on->task== NULL at times, but that, as !pendowner->pi_blocked_on, seems to be fallout. The start of bad juju is alwayspi_blocked_on!= waiter.[ 141.609268] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 141.609268] IP: [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] PGD 20e174067 PUD 20e253067 PMD 0 [ 141.609268] Oops: 0000 [#1] PREEMPT SMP [ 141.609268] last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map [ 141.609268] CPU 0 [ 141.609268] Pid: 8154, comm: pthread_cond_ma Tainted: G W 2.6.33.6-rt23 #12 MS-7502/MS-7502 [ 141.609268] RIP: 0010:[<ffffffff8106856d>] [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP: 0018:ffff88020e3cdd78 EFLAGS: 00010097 [ 141.609268] RAX: 0000000000000000 RBX: ffff8801e8eba5c0 RCX: 0000000000000000 [ 141.609268] RDX: ffff880028200000 RSI: 0000000000000046 RDI: 0000000000000009 [ 141.609268] RBP: ffff88020e3cdda8 R08: 0000000000000002 R09: 0000000000000000 [ 141.609268] R10: 0000000000000005 R11: 0000000000000000 R12: ffffffff81659068 [ 141.609268] R13: ffff8801e8ebdb58 R14: 0000000000000000 R15: ffff8801e8ebac08 [ 141.609268] FS: 00007f664d539700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 [ 141.609268] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.609268] CR2: 0000000000000058 CR3: 0000000214266000 CR4: 00000000000006f0 [ 141.609268] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.609268] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 141.609268] Process pthread_cond_ma (pid: 8154, threadinfo ffff88020e3cc000, task ffff88020e2a4700) [ 141.609268] Stack: [ 141.609268] 0000000000000000 ffffffff81659068 0000000000000202 0000000000000000 [ 141.609268]<0> 0000000000000000 0000000080001fda ffff88020e3cddc8 ffffffff812fec48 [ 141.609268]<0> ffffffff81659068 0000000000606300 ffff88020e3cddd8 ffffffff812ff1b9 [ 141.609268] Call Trace: [ 141.609268] [<ffffffff812fec48>] rt_spin_lock_slowunlock+0x43/0x61 [ 141.609268] [<ffffffff812ff1b9>] rt_spin_unlock+0x46/0x48 [ 141.609268] [<ffffffff81067d7f>] do_futex+0x83c/0x935 [ 141.609268] [<ffffffff810c26ce>] ? handle_mm_fault+0x6de/0x6f1 [ 141.609268] [<ffffffff81067e36>] ? do_futex+0x8f3/0x935 [ 141.609268] [<ffffffff81067fba>] sys_futex+0x142/0x154 [ 141.609268] [<ffffffff81020eb0>] ? do_page_fault+0x23e/0x28e [ 141.609268] [<ffffffff81004aa7>] ? math_state_restore+0x3d/0x3f [ 141.609268] [<ffffffff81004b08>] ? do_device_not_available+0xe/0x12 [ 141.609268] [<ffffffff81002c5b>] system_call_fastpath+0x16/0x1b [ 141.609268] Code: c7 09 6d 41 81 e8 ac 34 fd ff 4c 39 ab 70 06 00 00 74 11 be 47 02 00 00 48 c7 c7 09 6d 41 81 e8 92 34 fd ff 48 8b 83 70 06 00 00<4c> 39 60 58 74 11 be 48 02 00 00 48 c7 c7 09 6d 41 81 e8 74 34 [ 141.609268] RIP [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP<ffff88020e3cdd78> [ 141.609268] CR2: 0000000000000058 [ 141.609268] ---[ end trace 58805b944e6f93ce ]--- [ 141.609268] note: pthread_cond_ma[8154] exited with preempt_count 2(5. eyeball locks.. -> zzzzt -> report -> eyeball..)
-Mike
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
Quoted 13 lines · Darren Hart
Quoted 4 lines · Thomas Gleixner
Cc'ing Darren.
On Wed, 7 Jul 2010, Mike Galbraith wrote:
Hi Mike,
Quoted 5 lines
Quoted 5 lines
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc> 4.3, else testcases will segfault due to a gcc bug.
Interesting, I had not hit any gcc specific issues with this. Can you point me to the bug?
gcc-4.3 doesn't grok volatile inside a struct. Workaround below.
testcases/realtime/include/librttest.h +1 -1
@@ -88,7 +88,7 @@ struct thread {int flags;int id; }; -typedef struct {volatile int counter; }atomic_t ; +typedef volatile struct {volatile int counter; }atomic_t ;// flags for threads #define THREAD_START 1
Quoted 10 lines
Quoted 8 lines
run.sh make · line 25
Quoted 8 lines
run.sh renamed from run.sh.org +1
-1@@ -22,7 +22,7 @@ make # process to run realtime. The remainder of the processes (if any) # will run non-realtime in any case. -nthread=5000 +nthread=500Was this just to lighten the load, or was it required to reproduce?
Just to lighten it.
Quoted 1 line
Which architecture?
x64_64.
Quoted 1 line
Glibc version?
glibc-2.11.2-2.3
Quoted 3 lines
I see kernel version is: 2.6.33.6-rt23, have you reproduced this on earlier kernel versions as well? Any 2.6.31 rt kernel would be a good data point.
Yes, both 2.6.31.14-rt21 and 2.6.29.6-rt24.
Quoted 1 line
Is this immediately reproducible for you?
Yes, nearly instant kaboom.
Quoted 3 lines
I see a possibly fault occuring in the stack, if you run with
mlockall(), does the problem go away? (assuming not, but an easy thing to test).
Nope, -m made no difference.
-Mike
Quoted 3 lines · Darren Hart
I see kernel version is: 2.6.33.6-rt23, have you reproduced this on earlier kernel versions as well? Any 2.6.31 rt kernel would be a good data point.
FUTEX_CMP/WAIT_REQUEUE_PI first appeared in 2.6.29-rt, as does the explosion.
-Mike
Quoted 9 lines · Mike Galbraith
Quoted 3 lines · Darren Hart
I see kernel version is: 2.6.33.6-rt23, have you reproduced this on earlier kernel versions as well? Any 2.6.31 rt kernel would be a good data point.
FUTEX_CMP/WAIT_REQUEUE_PI first appeared in 2.6.29-rt, as does the explosion.
Right. I haven't had a chance to look into this yet. I will have some time today.
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
Quoted 8 lines · Thomas Gleixner
Cc'ing Darren.
Quoted 4 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
<snip>
embarrassing ltp realtime/perf/latency/pthread_cond_many breakage </snip>
Quoted 6 lines
Quoted 6 lines
4. run it.
What happens here is we hit
WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comesWARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.
So the WARN_ON sequence is obviously wrong, if it's critical it should be a BUG(), if not we shouldn't dereference what we know to be null. The following patch avoids the NULL pointer dereference in the WARN_ON. With this patch the NULL WARN_ON makes it to the console, and test runs to completion with no obvious negative side effects. I'm only posting for reference at this point, as while this may be necessary, it isn't the right "solution".
Some other data points from what time I could spend on this today.
FC12 kernel (2.6.31 based) has Requeue PI support, but does not exhibit this behavior.
2.6.33.5-rt23 without CONFIG_PREEMPT_RT does NOT exhibit this behavior.
2.6.33.5-rt23 does exhibit this behavior.
The minimal tracing I attempted (a handful of trace_printk's and run with the nop plugin) all prevented the crash from happening.
There appears to be no correlation to pi_blocked_on being NULL and the next pointer being NULL (I saw a roughly equivalent mix of NULL and valid pointers for next when pi_blocked_on was NULL).
Tonight/Tomorrow I'll review the rtmutex and futex code to try and fully understand (again) the usage of pi_blocked_on and if we need to avoid this scenario, or if we need to handle it "gracefully".
Quoted 1 line
From
fa6a6bee6e467d12d3774612c838703acd265ea6Mon Sep 17 00:00:00 2001
From: Darren Hart <[email protected]>
Date: Thu, 8 Jul 2010 19:44:35 -0400
Subject: [PATCH] rtmutex: avoid warnon bug
--- kernel/rtmutex.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
kernel/rtmutex.c +2 -2
@@ -579,9 +579,9 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)raw_spin_lock (&pendowner->pi_lock); -WARN_ON (!pendowner->pi_blocked_on);WARN_ON (pendowner->pi_blocked_on != waiter); -WARN_ON (pendowner->pi_blocked_on->lock != lock); +if (!WARN_ON (!pendowner->pi_blocked_on)) +WARN_ON (pendowner->pi_blocked_on->lock != lock); pendowner->pi_blocked_on =NULL ;
Signature · 7 lines
1.6.5.2 -- Darren Hart IBM Linux Technology Center Real-Time Linux Team
Quoted 7 lines · Darren Hart
So the
WARN_ONsequence is obviously wrong, if it's critical it should be aBUG(), if not we shouldn't dereference what we know to be null. The following patch avoids the NULL pointer dereference in theWARN_ON. With this patch the NULLWARN_ONmakes it to the console, and test runs to completion with no obvious negative side effects. I'm only posting for reference at this point, as while this may be necessary, it isn't the right "solution".
I've been slogging through the locking under the assumption that pi_blocked_on->task pointing to a stranger is very bad juju, but you're right, the only obviously evil consequence I see is tripping over the fallout in WARN_ON().
Quoted 3 lines
Tonight/Tomorrow I'll review the rtmutex and futex code to try and fully understand (again) the usage of
pi_blocked_onand if we need to avoid this scenario, or if we need to handle it "gracefully".
I hope you find it, I'm going blind crawling in endless circles :)
-Mike
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 3 lines · Darren Hart
I'll keep on looking, but I wanted to share what I've learned in case anyone in another timezone is hoping to continue debugging should I fall asleep at the helm ;-)
Still making smoke here, no heat or light tho..
Q: why is it ok to call rt_mutex_proxy_unlock() without the wait_lock held? Every other path leading to rt_mutex_set_owner() seems to hold the lock. Locking it did diddly spit other than shut up my WARN_ON().
kernel/futex.c +2 -0
@@ -407,7 +407,9 @@ static void free_pi_state(struct futex_pi_state *pi_state)list_del_init (&pi_state->list);raw_spin_unlock_irq (&pi_state->owner->pi_lock); +raw_spin_lock (&pi_state->pi_mutex.wait_lock);rt_mutex_proxy_unlock (&pi_state->pi_mutex, pi_state->owner); +raw_spin_unlock (&pi_state->pi_mutex.wait_lock); }if (current->pi_state_cache)
Quoted 7 lines · Darren Hart
Walking through it:
First the dumps:
------------[ cut here ]------------ WARNING: at kernel/rtmutex.c:583 wakeup_next_waiter+0x1ad/0x220()
Quoted 5 lines
WARN_ON(pendowner->pi_blocked_on != waiter);
Thepi_blocked_onis not NULL, but it isn't the expected waiter either. This means that the top waiter selected at the beginning ofwakeup_next_waiter()is now blocked on a lock with a different waiter structure, possibly on a different lock.
pendowner->pi_blocked_on changes while we're in wakeup_next_waiter(). The below fi^Wmade it not do that any more. We hold the wait_lock for this lock, but if the wakee blocks on another, what's protecting us?
bandaid-by: /me
kernel/rtmutex.c +2 -1
@@ -525,6 +525,8 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate) pendowner = waiter->task; waiter->task =NULL ; +raw_spin_lock (&pendowner->pi_lock); +/* * Do the wakeup before the ownership change to give any spinning * waiter grantees a headstart over the other threads that will
@@ -577,8 +579,6 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)else next =NULL ; -raw_spin_lock (&pendowner->pi_lock);
This would likely be better if holding over wakeup is ok.
pi-tests still work, as does everything else I've tried. Doesn't _seem_ like a bad idea to keep pending owner on a leash until we're done fiddling with him, but dunno, I've never needed to tinker here.
kernel/rtmutex.c +3 -3
@@ -525,6 +525,9 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate) pendowner = waiter->task; waiter->task =NULL ; +raw_spin_unlock (¤t->pi_lock); +raw_spin_lock (&pendowner->pi_lock); +/* * Do the wakeup before the ownership change to give any spinning * waiter grantees a headstart over the other threads that will
@@ -562,8 +565,6 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)rt_mutex_set_owner (lock, pendowner,RT_MUTEX_OWNER_PENDING ); -raw_spin_unlock (¤t->pi_lock); -/* * Clear the pi_blocked_on variable and enqueue a possible * waiter into the pi_waiters list of the pending owner. This
@@ -577,8 +578,6 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)else next =NULL ; -raw_spin_lock (&pendowner->pi_lock);
Quoted 15 lines · Mike Galbraith
Quoted 7 lines · Darren Hart
Walking through it:
First the dumps:
------------[ cut here ]------------ WARNING: at kernel/rtmutex.c:583 wakeup_next_waiter+0x1ad/0x220()Quoted 5 lines
WARN_ON(pendowner->pi_blocked_on != waiter);
Thepi_blocked_onis not NULL, but it isn't the expected waiter either. This means that the top waiter selected at the beginning ofwakeup_next_waiter()is now blocked on a lock with a different waiter structure, possibly on a different lock.
Quoted 3 lines
pendowner->pi_blocked_onchanges while we're inwakeup_next_waiter(). The below fi^Wmade it not do that any more. We hold thewait_lockfor this lock, but if the wakee blocks on another, what's protecting us?
If pendowner is blocked on "lock" to begin with (he should be as his waiter struct in in the rtmutex waiters list) then he can't block on someone else until he either acquires this one or removes himself as a waiter (due to a timeout for instance) - both of these operations require holding lock->wait_lock, which is held by the caller of wakeup_next_waiter().
Seems more likely that the below forces a missing memory barrier... not sure yet though. Good data point.
--
Darren
Quoted 27 lines
bandaid-by: /me
kernel/rtmutex.c +2
-2@@ -525,6 +525,8 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate) pendowner = waiter->task; waiter->task =NULL ; +raw_spin_lock (&pendowner->pi_lock); +/* * Do the wakeup before the ownership change to give any spinning * waiter grantees a headstart over the other threads that will @@ -577,8 +579,6 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)else next =NULL ; -raw_spin_lock (&pendowner->pi_lock); -WARN_ON (!pendowner->pi_blocked_on);WARN_ON (pendowner->pi_blocked_on != waiter);WARN_ON (pendowner->pi_blocked_on->lock != lock);
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
Quoted 6 lines · Darren Hart
If pendowner is blocked on "lock" to begin with (he should be as his waiter struct in in the rtmutex waiters list) then he can't block on someone else until he either acquires this one or removes himself as a waiter (due to a timeout for instance) - both of these operations require holding
lock->wait_lock, which is held by the caller ofwakeup_next_waiter().
Hm, right, first thing wakee does is re-grab the wait_lock.
-Mike
Quoted 65 lines · Thomas Gleixner
Cc'ing Darren.
Quoted 61 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
4. run it.What happens here is we hit
WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comesWARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.With explosion avoidance, I also see
pendowner->pi_blocked_on->task== NULL at times, but that, as !pendowner->pi_blocked_on, seems to be fallout. The start of bad juju is alwayspi_blocked_on!= waiter.[ 141.609268] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 141.609268] IP: [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] PGD 20e174067 PUD 20e253067 PMD 0 [ 141.609268] Oops: 0000 [#1] PREEMPT SMP [ 141.609268] last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map [ 141.609268] CPU 0 [ 141.609268] Pid: 8154, comm: pthread_cond_ma Tainted: G W 2.6.33.6-rt23 #12 MS-7502/MS-7502 [ 141.609268] RIP: 0010:[<ffffffff8106856d>] [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP: 0018:ffff88020e3cdd78 EFLAGS: 00010097 [ 141.609268] RAX: 0000000000000000 RBX: ffff8801e8eba5c0 RCX: 0000000000000000 [ 141.609268] RDX: ffff880028200000 RSI: 0000000000000046 RDI: 0000000000000009 [ 141.609268] RBP: ffff88020e3cdda8 R08: 0000000000000002 R09: 0000000000000000 [ 141.609268] R10: 0000000000000005 R11: 0000000000000000 R12: ffffffff81659068 [ 141.609268] R13: ffff8801e8ebdb58 R14: 0000000000000000 R15: ffff8801e8ebac08 [ 141.609268] FS: 00007f664d539700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 [ 141.609268] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.609268] CR2: 0000000000000058 CR3: 0000000214266000 CR4: 00000000000006f0 [ 141.609268] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.609268] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 141.609268] Process pthread_cond_ma (pid: 8154, threadinfo ffff88020e3cc000, task ffff88020e2a4700) [ 141.609268] Stack: [ 141.609268] 0000000000000000 ffffffff81659068 0000000000000202 0000000000000000 [ 141.609268]<0> 0000000000000000 0000000080001fda ffff88020e3cddc8 ffffffff812fec48 [ 141.609268]<0> ffffffff81659068 0000000000606300 ffff88020e3cddd8 ffffffff812ff1b9 [ 141.609268] Call Trace: [ 141.609268] [<ffffffff812fec48>] rt_spin_lock_slowunlock+0x43/0x61 [ 141.609268] [<ffffffff812ff1b9>] rt_spin_unlock+0x46/0x48 [ 141.609268] [<ffffffff81067d7f>] do_futex+0x83c/0x935 [ 141.609268] [<ffffffff810c26ce>] ? handle_mm_fault+0x6de/0x6f1 [ 141.609268] [<ffffffff81067e36>] ? do_futex+0x8f3/0x935 [ 141.609268] [<ffffffff81067fba>] sys_futex+0x142/0x154 [ 141.609268] [<ffffffff81020eb0>] ? do_page_fault+0x23e/0x28e [ 141.609268] [<ffffffff81004aa7>] ? math_state_restore+0x3d/0x3f [ 141.609268] [<ffffffff81004b08>] ? do_device_not_available+0xe/0x12 [ 141.609268] [<ffffffff81002c5b>] system_call_fastpath+0x16/0x1b [ 141.609268] Code: c7 09 6d 41 81 e8 ac 34 fd ff 4c 39 ab 70 06 00 00 74 11 be 47 02 00 00 48 c7 c7 09 6d 41 81 e8 92 34 fd ff 48 8b 83 70 06 00 00<4c> 39 60 58 74 11 be 48 02 00 00 48 c7 c7 09 6d 41 81 e8 74 34 [ 141.609268] RIP [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP<ffff88020e3cdd78> [ 141.609268] CR2: 0000000000000058 [ 141.609268] ---[ end trace 58805b944e6f93ce ]--- [ 141.609268] note: pthread_cond_ma[8154] exited with preempt_count 2(5. eyeball locks.. -> zzzzt -> report -> eyeball..)
-Mike
OK, here is the final analysis. After a trace-cmd fix and some rtmutex "paradigm" insight from Rostedt, the root of the problem came out:
Jul 9 14:26:39 elm9m94 kernel: ------------[ cut here ]------------ Jul 9 14:26:39 elm9m94 kernel: WARNING: at kernel/rtmutex.c:590 wakeup_next_waiter+0x244/0x370() Jul 9 14:26:39 elm9m94 kernel: Hardware name: IBM eServer BladeCenter HS21 -[7995AC1]- Jul 9 14:26:39 elm9m94 kernel: Modules linked in: scsi_wait_scan Jul 9 14:26:39 elm9m94 kernel: Pid: 3870, comm: pthread_cond_ma Not tainted 2.6.33.5-rt23dvh01 #13 Jul 9 14:26:39 elm9m94 kernel: Call Trace: Jul 9 14:26:39 elm9m94 kernel: [<ffffffff8104dc3b>] warn_slowpath_common+0x7b/0xc0 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff8104dc94>] warn_slowpath_null+0x14/0x20 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff81083f74>] wakeup_next_waiter+0x244/0x370 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff81921875>] rt_mutex_slowunlock+0x35/0x80 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff819218e9>] rt_mutex_unlock+0x29/0x40 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff8108258c>] do_futex+0xa6c/0xae0
# addr2line -e vmlinux ffffffff8108258c /test/dvhart/source/linux-2.6-tip.git/kernel/futex.c:833 In futex_unlock_pi() unlocking the pi futex backing the pthread_mutex:
rt_mutex_unlock(&pi_state->pi_mutex);
and a trace of the pending owner with the changing waiter (pi_blocked_on) from sched_show_task():
Jul 9 14:26:39 elm9m94 kernel: pthread_cond_ M ffffffff81922537 0 4176 3745 0x00000080 Jul 9 14:26:39 elm9m94 kernel: ffffffff821fc9a8 0000000000000000 ffff88042ded9c08 ffffffff819216c0 Jul 9 14:26:39 elm9m94 kernel: 0000000000000000 ffff88042dbbabc0 ffffffff821fc9c0 ffff88042decee40 Jul 9 14:26:39 elm9m94 kernel: ffff88042decee40 ffff88042decee40 ffff880400000000 ffff88042ded9b70 Jul 9 14:26:39 elm9m94 kernel: Call Trace: Jul 9 14:26:39 elm9m94 kernel: [<ffffffff819216c0>] ? rt_spin_lock_slowlock+0x190/0x310 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff8192216c>] ? rt_spin_lock+0x7c/0x90 Jul 9 14:26:39 elm9m94 kernel: [<ffffffff810810c8>] ? futex_wait_requeue_pi+0x178/0x360
# addr2line -e vmlinux ffffffff810810c8 /test/dvhart/source/linux-2.6-tip.git/kernel/futex.c:153
That's inside match_futex(), back up a line:
# addr2line -e vmlinux ffffffff810810c0 /test/dvhart/source/linux-2.6-tip.git/kernel/futex.c:2258
This is futex_wait_requeue_pi():
/* Queue the futex_q, drop the hb lock, wait for wakeup. */
futex_wait_queue_me(hb, &q, to);
spin_lock(&hb->lock);
ret = handle_early_requeue_pi_wakeup(hb, &q, &key2, to);
spin_unlock(&hb->lock);Snippets from the trace:
pthread_cond_ma-4176 [003] 395.357310: bprint: task_blocks_on_rt_mutex : 4176 blocks on waiter: pthread_cond_ma-4176 [003] 395.357310: bprint: task_blocks_on_rt_mutex : waiter: 0xffff88042ded9b68 pthread_cond_ma-4176 [003] 395.357311: bprint: task_blocks_on_rt_mutex : waiter->lock: 0xffffffff821fc9a8 pthread_cond_ma-4176 [003] 395.357311: bprint: task_blocks_on_rt_mutex : was: 0xffff88042ded9cc8
^ should always be NULL
pthread_cond_ma-4176 [003] 395.357311: bprint: task_blocks_on_rt_mutex : oldwaiter->lock: 0xffff8804104d0450
pthread_cond_ma-3870 [000] 395.357732: bprint: wakeup_next_waiter : BUG: waiter changed pthread_cond_ma-3870 [000] 395.357733: bprint: wakeup_next_waiter : pendowner pid: 4176 pthread_cond_ma-3870 [000] 395.357734: bprint: wakeup_next_waiter : pendowner comm: pthread_cond_ma-3870 [000] 395.357734: bprint: wakeup_next_waiter : waiter->lock: 0xffff8804104d0450 pthread_cond_ma-3870 [000] 395.357734: bprint: wakeup_next_waiter : pi_blocked_on->lock: 0xffffffff821fc9a8
The one it changed to is the one we see in the task_blocks_on_rt_mutex above. It has woken from the futex_wait_queue_me() after having been requeued to the pi futex (we know this because it wouldn't have a pi_blocked_on if it was still on the wait queue futex. Also the futex_unlock_pi() call by 3870 is trying to unlock the previous lock 4167 was locked on.
The core of the problem is that the proxy_lock blocks a task on a lock the task knows nothing about. So when it wakes up inside of futex_wait_requeue_pi, it immediately tries to block on hb->lock to check why it woke up. This has the potential to block the task on two locks (thus overwriting the pi_blocked_on). Any attempt preventing this involves a lock, and ultimiately the hb->lock. The only solution I see is to make the hb->locks raw locks (thanks to Steven Rostedt for original idea and batting this around with me in IRC).
Given the contention the hb->locks can be under, I'm concerned about the latency impacts this can have, still, I don't see another way to allow for the proxy locking that the rtmutex semantics require for requeue_pi to work. I'll work up a patch and do some testing to see if I can get a feel for the impact.
Ugh.
I'm still hoping to find some clever way to avoid this, going to map out this state machine again and see if there is a way around it...
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
PATCH 1/1 futex: protect against pi_blocked_on corruption during requeue PI
In reply to Thomas GleixnerThanks to Thomas, Steven, and Mike for hashing this over me. After an IRC discussion with Thomas, I put the following together. It resolves the issue for me, Mike please test and let us know if it fixes it for you. A couple of points of discussion before we commit this:
The use of the new state flag, PI_WAKEUP_INPROGRESS, is pretty ugly. Would a new task_pi_blocked_on_valid() method be preferred (in rtmutex.c)?
The new WARN_ON() in task_blocks_on_rt_mutex() is complex. It didn't exist before and we've now closed this gap, should we just drop it?
I've added a couple BUG_ON()s in futex_wait_requeue_pi() dealing with the race with requeue and q.lock_ptr. I'd like to leave this for the time being if nobody strongly objects.
Thanks,
Darren
Quoted 1 line
From
93fd3bb97800ebf5e5c1a6a85937bab93256dd42Mon Sep 17 00:00:00 2001
From: Darren Hart <[email protected]>
Date: Fri, 9 Jul 2010 17:50:23 -0400
Subject: [PATCH 1/2] futex: protect against pi_blocked_on corruption during requeue PI
The requeue_pi mechanism introduced proxy locking of the rtmutex. This creates a scenario where a task can wakeup, not knowing it has been enqueued on an rtmutex. Blocking on an hb->lock() can overwrite a valid value in current->pi_blocked_on, leading to an inconsistent state.
Prevent overwriting pi_blocked_on by serializing on the waiter's pi_lock (a raw_spinlock) and using the new PI_WAKEUP_INPROGRESS state flag to indicate a waiter that has been woken by a timeout or signal. This prevents the rtmutex code from adding the waiter to the rtmutex wait list, returning EAGAIN to futex_requeue(), which will in turn ignore the waiter during a requeue. Care is taken to allow current to block on locks even if PI_WAKEUP_INPROGRESS is set.
During normal wakeup, this results in one less hb->lock protected section. In the pre-requeue-timeout-or-signal wakeup, this removes the "greedy locking" behavior, no attempt will be made to acquire the lock.
--- kernel/futex.c | 50 +++++++++++++++++++++++++++++++++------------- kernel/rtmutex.c | 45 ++++++++++++++++++++++++++++++++++------- kernel/rtmutex_common.h | 1 + kernel/sched.c | 5 +++- 4 files changed, 78 insertions(+), 23 deletions(-)
kernel/futex.c +36 -14
@@ -1336,6 +1336,9 @@ retry_private:requeue_pi_wake_futex (this, &key2, hb2); drop_count++;continue ; + }else if (ret == -EAGAIN ) { +/* Waiter woken by timeout or signal. */ +continue ; }else if (ret) {/* -EDEADLK */ this->pi_state =NULL ;
@@ -2211,9 +2214,9 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared, int clockrt, u32 __user *uaddr2) { struct hrtimer_sleeper timeout, *to = NULL; +struct futex_hash_bucket *hb, *hb2;struct rt_mutex_waiter rt_waiter;struct rt_mutex *pi_mutex =NULL ; -struct futex_hash_bucket *hb;union futex_key key2;struct futex_q q;int res, ret;
@@ -2255,18 +2258,33 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared,/* Queue the futex_q, drop the hb lock, wait for wakeup. */ futex_wait_queue_me (hb, &q, to); -spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); -spin_unlock (&hb->lock); -if (ret) - goto out_put_keys; -/* -* In order for us to be here, we know our q.key == key2, and since -* we took the hb->lock above, we also know that futex_requeue() has -* completed and we no longer have to concern ourselves with a wakeup -* race with the atomic proxy lock acquition by the requeue code. +* Avoid races with requeue and trying to block on two mutexes +* (hb->lock and uaddr2's rtmutex) by serializing access to +* pi_blocked_on with pi_lock and setting PI_BLOCKED_ON_PENDING. +*/ +raw_spin_lock (¤t->pi_lock); +if (current->pi_blocked_on) { +raw_spin_unlock (¤t->pi_lock); + }else { + current->pi_blocked_on = (struct rt_mutex_waiter *)PI_WAKEUP_INPROGRESS ; +raw_spin_unlock (¤t->pi_lock); + +spin_lock (&hb->lock); + ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); +spin_unlock (&hb->lock); +if (ret) + goto out_put_keys; + } + +/* +* In order to be here, we have either been requeued, are in the process +* of being requeued, or requeue successfully acquired uaddr2 on our +* behalf. If pi_blocked_on was non-null above, we may be racing with a +* requeue. Do not rely on q->lock_ptr to be hb2->lock until after +* blocking on hb->lock or hb2->lock. */ + hb2 =hash_futex (&key2);/* Check if the requeue code acquired the second futex for us. */ if (!q.rt_waiter) {
@@ -2275,10 +2293,12 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared,* did a lock-steal - fix up the PI-state in that case. */ if (q.pi_state && (q.pi_state->owner != current)) { -spin_lock (q.lock_ptr); +spin_lock (&hb2->lock); +BUG_ON (&hb2->lock != q.lock_ptr); + ret =fixup_pi_state_owner (uaddr2, &q, current, fshared); -spin_unlock (q.lock_ptr); +spin_unlock (&hb2->lock); } }else {/*
@@ -2291,7 +2311,9 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared, ret =rt_mutex_finish_proxy_lock (pi_mutex, to, &rt_waiter,1 );debug_rt_mutex_free_waiter (&rt_waiter); -spin_lock (q.lock_ptr); +spin_lock (&hb2->lock); +BUG_ON (&hb2->lock != q.lock_ptr); +/* * Fixup the pi_state owner and possibly acquire the lock if we * haven't already.
kernel/rtmutex.c +37 -8
93 lines not on this page · raw message
kernel/rtmutex_common.h +1 -0
@@ -115,6 +115,7 @@ static inline unsigned long rt_mutex_owner_pending(struct rt_mutex *lock)/* * PI-futex support (proxy locking functions, etc.): */ +#define PI_WAKEUP_INPROGRESS 1extern struct task_struct *rt_mutex_next_owner (struct rt_mutex *lock);extern void rt_mutex_init_proxy_locked (struct rt_mutex *lock,struct task_struct *proxy_owner);
kernel/sched.c +4 -1
@@ -83,6 +83,8 @@#define CREATE_TRACE_POINTS #include <trace/events/sched.h> +#include "rtmutex_common.h" +/* * Convert user-nice values [ -20 ... 0 ... 19 ] * to static priority [ MAX_RT_PRIO..MAX_PRIO-1 ],
@@ -6377,7 +6379,8 @@ void task_setprio(struct task_struct *p, int prio)*/ if (unlikely (p == rq->idle)) {WARN_ON (p != rq->curr); -WARN_ON (p->pi_blocked_on); +WARN_ON (p->pi_blocked_on && + (long )p->pi_blocked_on !=PI_WAKEUP_INPROGRESS ); goto out_unlock; }
Signature · 1 line
1.7.0.4
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI
Quoted 11 lines · Darren Hart
Thanks to Thomas, Steven, and Mike for hashing this over me. After an IRC discussion with Thomas, I put the following together. It resolves the issue for me, Mike please test and let us know if it fixes it for you. A couple of points of discussion before we commit this:
The use of the new state flag,
PI_WAKEUP_INPROGRESS, is pretty ugly. Would a newtask_pi_blocked_on_valid()method be preferred (inrtmutex.c)?The new
WARN_ON()intask_blocks_on_rt_mutex()is complex. It didn't exist before and we've now closed this gap, should we just drop it?
We can simplify it to:
WARN_ON(task->pi_blocked_on && task->pi_blocked_on != PI_WAKEUP_INPROGRESS);
We check for !=current and PI_WAKEUP_INPROGRESS just above.
Quoted 4 lines
I've added a couple BUG_ON()s in
futex_wait_requeue_pi()dealing with the race with requeue and q.lock_ptr. I'd like to leave this for the time being if nobody strongly objects.kernel/futex.c +0
-1-
kernel/futex.c static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared, · line 2266
Quoted 10 lines
kernel/futex.c +5
-4/* -* In order for us to be here, we know our q.key == key2, and since -* we took the hb->lock above, we also know that futex_requeue() has -* completed and we no longer have to concern ourselves with a wakeup -* race with the atomic proxy lock acquition by the requeue code. +* Avoid races with requeue and trying to block on two mutexes +* (hb->lock and uaddr2's rtmutex) by serializing access to +* pi_blocked_on with pi_lock and setting PI_BLOCKED_ON_PENDING. +*/ +raw_spin_lock (¤t->pi_lock);
Needs to be raw_spin_lock_irq()
kernel/futex.c static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared, · line 2270
Quoted 4 lines
kernel/futex.c +4
-0+if (current->pi_blocked_on) { +raw_spin_unlock (¤t->pi_lock); + }else { + current->pi_blocked_on = (struct rt_mutex_waiter *)PI_WAKEUP_INPROGRESS ;
#define PI_WAKEUP_INPROGRESS ((struct rt_mutex_waiter *) 1)
perhaps ? That gets rid of all type casts
kernel/futex.c static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared, · line 2273
Quoted 3 lines
kernel/futex.c +3
-0+raw_spin_unlock (¤t->pi_lock); + +spin_lock (&hb->lock);
We need to cleanup current->pi_blocked_on here. If we succeed in the hb->lock fast path then we might leak the PI_WAKEUP_INPROGRESS to user space and the next requeue will fail.
kernel/rtmutex.c static int rt_mutex_adjust_prio_chain(struct task_struct *task, · line 231
Quoted 11 lines
kernel/rtmutex.c +1
-1@@ -227,7 +227,7 @@ static int rt_mutex_adjust_prio_chain(struct task_struct *task,* reached or the state of the chain has changed while we * dropped the locks. */ -if (!waiter || !waiter->task) +if (!waiter || (long )waiter ==PI_WAKEUP_INPROGRESS || !waiter->task) goto out_unlock_pi;
Why do we need that check ? Either the requeue succeeded then task->pi_blocked_on is set to the real waiter or the wakeup won and we are in no lock chain.
If we ever find a waiter with PI_WAKEUP_INPROGRESS set in rt_mutex_adjust_prio_chain() then it's a bug nothing else.
kernel/rtmutex.c static int task_blocks_on_rt_mutex(struct rt_mutex *lock, · line 497
Quoted 7 lines
kernel/rtmutex.c +2
-1@@ -469,7 +493,8 @@ static int task_blocks_on_rt_mutex(struct rt_mutex *lock,plist_add (&waiter->pi_list_entry, &owner->pi_waiters);__rt_mutex_adjust_prio (owner); -if (owner->pi_blocked_on) +if (owner->pi_blocked_on && + (long )owner->pi_blocked_on !=PI_WAKEUP_INPROGRESS )
Again, that can never happen
kernel/rtmutex.c static void wakeup_next_waiter(struct rt_mutex *lock, int savestate) · line 608
Quoted 12 lines
kernel/rtmutex.c +2
-3chain_walk =1 ;raw_spin_unlock (&owner->pi_lock); }@@ -579,9 +604,11 @@ static void wakeup_next_waiter(struct rt_mutex *lock, int savestate)raw_spin_lock (&pendowner->pi_lock); -WARN_ON (!pendowner->pi_blocked_on); -WARN_ON (pendowner->pi_blocked_on != waiter); -WARN_ON (pendowner->pi_blocked_on->lock != lock); +if (!WARN_ON (!pendowner->pi_blocked_on) && + !WARN_ON ((long )pendowner->pi_blocked_on ==PI_WAKEUP_INPROGRESS )) {
Ditto
Quoted 7 lines
kernel/rtmutex.c +3
-0+WARN_ON (pendowner->pi_blocked_on != waiter); +WARN_ON (pendowner->pi_blocked_on->lock != lock); + } pendowner->pi_blocked_on =NULL ;
kernel/rtmutex.c static void remove_waiter(struct rt_mutex *lock, · line 656
Quoted 7 lines
kernel/rtmutex.c +2
-1@@ -624,7 +651,8 @@ static void remove_waiter(struct rt_mutex *lock, }__rt_mutex_adjust_prio (owner); -if (owner->pi_blocked_on) +if (owner->pi_blocked_on && + (long )owner->pi_blocked_on !=PI_WAKEUP_INPROGRESS ) chain_walk =1 ;
Same here.
kernel/rtmutex.c void rt_mutex_adjust_pi(struct task_struct *task) · line 690
Quoted 8 lines
kernel/rtmutex.c +2
-1raw_spin_unlock (&owner->pi_lock);@@ -658,7 +686,8 @@ void rt_mutex_adjust_pi(struct task_struct *task)raw_spin_lock_irqsave (&task->pi_lock, flags); waiter = task->pi_blocked_on; -if (!waiter || waiter->list_entry.prio == task->prio) { +if (!waiter || (long )waiter ==PI_WAKEUP_INPROGRESS || + waiter->list_entry.prio == task->prio) {
And here
kernel/rtmutex.c void task_setprio(struct task_struct *p, int prio) · line 6383
Quoted 10 lines
kernel/rtmutex.c +2
-1/* * Convert user-nice values [ -20 ... 0 ... 19 ] * to static priority [ MAX_RT_PRIO..MAX_PRIO-1 ], @@ -6377,7 +6379,8 @@ void task_setprio(struct task_struct *p, int prio)*/ if (unlikely (p == rq->idle)) {WARN_ON (p != rq->curr); -WARN_ON (p->pi_blocked_on); +WARN_ON (p->pi_blocked_on && + (long )p->pi_blocked_on !=PI_WAKEUP_INPROGRESS );
Yuck. Paranoia ? If we ever requeue idle, then .....
I'm going to cleanup the stuff and send out a new patch for Mike to test.
Thanks,
tglx
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI
Quoted 16 lines · Thomas Gleixner
kernel/rtmutex.c static int rt_mutex_adjust_prio_chain(struct task_struct *task, · line 231
Quoted 9 lines · Darren Hart
kernel/rtmutex.c +1
-1@@ -227,7 +227,7 @@ static int rt_mutex_adjust_prio_chain(struct task_struct *task,* reached or the state of the chain has changed while we * dropped the locks. */ -if (!waiter || !waiter->task) +if (!waiter || (long )waiter ==PI_WAKEUP_INPROGRESS || !waiter->task) goto out_unlock_pi;Why do we need that check ? Either the requeue succeeded then
task->pi_blocked_onis set to the real waiter or the wakeup won and we are in no lock chain.
Quoted 2 lines
If we ever find a waiter with
PI_WAKEUP_INPROGRESSset inrt_mutex_adjust_prio_chain()then it's a bug nothing else.
Grrr, I'm wrong. If we take hb->lock in the fast path then something else might try to boost us and trip over this :(
This code causes braindamage. I really wonder whether we need to remove it according to the "United Nations Convention against Torture and Other Cruel, Inhuman or Degrading Treatment or Punishment".
Quoted 9 lines
kernel/rtmutex.c void task_setprio(struct task_struct *p, int prio) · line 6383
Quoted 7 lines
kernel/rtmutex.c +2
-1@@ -6377,7 +6379,8 @@ void task_setprio(struct task_struct *p, int prio)*/ if (unlikely (p == rq->idle)) {WARN_ON (p != rq->curr); -WARN_ON (p->pi_blocked_on); +WARN_ON (p->pi_blocked_on && + (long )p->pi_blocked_on !=PI_WAKEUP_INPROGRESS );Yuck. Paranoia ? If we ever requeue idle, then .....
At least one which is bogus :)
Thanks,
tglx
PATCH 1/1 futex: protect against pi_blocked_on corruption during requeue PI -V2
Quoted 5 lines · Thomas Gleixner
This code causes braindamage. I really wonder whether we need to remove it according to the "United Nations Convention against Torture and Other Cruel, Inhuman or Degrading Treatment or Punishment".
Ok, finally managed to twist my brain around it. Mike, can you give it a test ride ?
Thanks,
tglx
-------->
Subject: futex: Protect against pi_blocked_on corruption during requeue PI
From: Darren Hart <[email protected]>
Date: Fri, 9 Jul 2010 17:50:23 -0400
The requeue_pi mechanism introduced proxy locking of the rtmutex. This creates a scenario where a task can wakeup, not knowing it has been enqueued on an rtmutex. Blocking on an hb->lock() can overwrite a valid value in current->pi_blocked_on, leading to an inconsistent state.
Prevent overwriting pi_blocked_on by serializing on the waiter's pi_lock and using the new PI_WAKEUP_INPROGRESS state flag to indicate a waiter that has been woken by a timeout or signal. This prevents the rtmutex code from adding the waiter to the rtmutex wait list, returning EAGAIN to futex_requeue(), which will in turn ignore the waiter during a requeue. Care is taken to allow current to block on locks even if PI_WAKEUP_INPROGRESS is set.
During normal wakeup, this results in one less hb->lock protected section. In the pre-requeue-timeout-or-signal wakeup, this removes the "greedy locking" behavior, no attempt will be made to acquire the lock.
[ tglx: take pi_lock with lock_irq(), removed paranoid warning,
plugged pi_state and pi_blocked_on leak, adjusted some comments ]
LKML-Reference: <[email protected]>
--- kernel/futex.c | 50 +++++++++++++++++++++++++++++++++------------- kernel/rtmutex.c | 45 ++++++++++++++++++++++++++++++++++------- kernel/rtmutex_common.h | 1 + kernel/sched.c | 5 +++- 4 files changed, 78 insertions(+), 23 deletions(-)
kernel/futex.c +60 -13
@@ -1336,6 +1336,16 @@ retry_private:requeue_pi_wake_futex (this, &key2, hb2); drop_count++;continue ; + }else if (ret == -EAGAIN ) { +/* +* Waiter was woken by timeout or +* signal and has set pi_blocked_on to +* PI_WAKEUP_INPROGRESS before we +* tried to enqueue it on the rtmutex. +*/ + this->pi_state =NULL ; +free_pi_state (pi_state); +continue ; }else if (ret) {/* -EDEADLK */ this->pi_state =NULL ;
@@ -2211,9 +2221,9 @@ static int futex_wait_requeue_pi(u32 __u int clockrt, u32 __user *uaddr2) { struct hrtimer_sleeper timeout, *to = NULL; +struct futex_hash_bucket *hb, *hb2;struct rt_mutex_waiter rt_waiter;struct rt_mutex *pi_mutex =NULL ; -struct futex_hash_bucket *hb;union futex_key key2;struct futex_q q;int res, ret;
@@ -2255,18 +2265,51 @@ static int futex_wait_requeue_pi(u32 __u/* Queue the futex_q, drop the hb lock, wait for wakeup. */ futex_wait_queue_me (hb, &q, to); -spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); -spin_unlock (&hb->lock); -if (ret) - goto out_put_keys; +/* +* Avoid races with requeue and trying to block on two mutexes +* (hb->lock and uaddr2's rtmutex) by serializing access to +* pi_blocked_on with pi_lock. +*/ +raw_spin_lock_irq (¤t->pi_lock); +if (current->pi_blocked_on) { +/* Requeue happened already */ +raw_spin_unlock_irq (¤t->pi_lock); + }else { +/* +* Setting pi_blocked_on to PI_WAKEUP_INPROGRESS +* prevents a concurrent requeue from enqueuein us on +* the uaddr2 rtmutex. After that we can safely +* acquire (and possibly block on) hb->lock. +*/ + current->pi_blocked_on =PI_WAKEUP_INPROGRESS ; +raw_spin_unlock_irq (¤t->pi_lock); + +spin_lock (&hb->lock); + +/* +* Clean up pi_blocked_on. We might leak it otherwise +* when we succeeded with the hb->lock in the fast +* path. +*/ +raw_spin_lock_irq (¤t->pi_lock); + current->pi_blocked_on =NULL ; +raw_spin_unlock_irq (¤t->pi_lock); + + ret =handle_early_requeue_pi_wakeup (hb, &q, &key2, to); +spin_unlock (&hb->lock); +if (ret) + goto out_put_keys; + }/* -* In order for us to be here, we know our q.key == key2, and since -* we took the hb->lock above, we also know that futex_requeue() has -* completed and we no longer have to concern ourselves with a wakeup -* race with the atomic proxy lock acquition by the requeue code. +* In order to be here, we have either been requeued, are in +* the process of being requeued, or requeue successfully +* acquired uaddr2 on our behalf. If pi_blocked_on was +* non-null above, we may be racing with a requeue. Do not +* rely on q->lock_ptr to be hb2->lock until after blocking on +* hb->lock or hb2->lock. */ + hb2 =hash_futex (&key2);/* Check if the requeue code acquired the second futex for us. */ if (!q.rt_waiter) {
@@ -2275,10 +2318,12 @@ static int futex_wait_requeue_pi(u32 __u* did a lock-steal - fix up the PI-state in that case. */ if (q.pi_state && (q.pi_state->owner != current)) { -spin_lock (q.lock_ptr); +spin_lock (&hb2->lock); +BUG_ON (&hb2->lock != q.lock_ptr); + ret =fixup_pi_state_owner (uaddr2, &q, current, fshared); -spin_unlock (q.lock_ptr); +spin_unlock (&hb2->lock); } }else {/*
@@ -2291,7 +2336,9 @@ static int futex_wait_requeue_pi(u32 __u ret =rt_mutex_finish_proxy_lock (pi_mutex, to, &rt_waiter,1 );debug_rt_mutex_free_waiter (&rt_waiter); -spin_lock (q.lock_ptr); +spin_lock (&hb2->lock); +BUG_ON (&hb2->lock != q.lock_ptr); +/* * Fixup the pi_state owner and possibly acquire the lock if we * haven't already.
kernel/rtmutex.c +28 -5
@@ -82,6 +82,11 @@ static void fixup_rt_mutex_waiters(strucclear_rt_mutex_waiters (lock); } +static inline int rt_mutex_real_waiter (struct rt_mutex_waiter *waiter) +{ +return waiter && waiter !=PI_WAKEUP_INPROGRESS ; +} +/* * We can speed up the acquire/release, if the architecture * supports cmpxchg and if there's no debugging state to be set up
@@ -227,7 +232,7 @@ static int rt_mutex_adjust_prio_chain(st* reached or the state of the chain has changed while we * dropped the locks. */ -if (!waiter || !waiter->task) +if (!rt_mutex_real_waiter (waiter) || !waiter->task) goto out_unlock_pi;/*
@@ -448,6 +453,23 @@ static int task_blocks_on_rt_mutex(strucint chain_walk =0 , res;raw_spin_lock (&task->pi_lock); + +/* +* In the case of futex requeue PI, this will be a proxy +* lock. The task will wake unaware that it is enqueueed on +* this lock. Avoid blocking on two locks and corrupting +* pi_blocked_on via the PI_WAKEUP_INPROGRESS +* flag. futex_wait_requeue_pi() sets this when it wakes up +* before requeue (due to a signal or timeout). Do not enqueue +* the task if PI_WAKEUP_INPROGRESS is set. +*/ +if (task != current && task->pi_blocked_on ==PI_WAKEUP_INPROGRESS ) { +raw_spin_unlock (&task->pi_lock); +return -EAGAIN ; + } + +BUG_ON (rt_mutex_real_waiter (task->pi_blocked_on)); +__rt_mutex_adjust_prio (task); waiter->task = task; waiter->lock = lock;
@@ -469,7 +491,7 @@ static int task_blocks_on_rt_mutex(strucplist_add (&waiter->pi_list_entry, &owner->pi_waiters);__rt_mutex_adjust_prio (owner); -if (owner->pi_blocked_on) +if (rt_mutex_real_waiter (owner->pi_blocked_on)) chain_walk =1 ;raw_spin_unlock (&owner->pi_lock); }
@@ -624,7 +646,7 @@ static void remove_waiter(struct rt_mute }__rt_mutex_adjust_prio (owner); -if (owner->pi_blocked_on) +if (rt_mutex_real_waiter (owner->pi_blocked_on)) chain_walk =1 ;raw_spin_unlock (&owner->pi_lock);
@@ -658,7 +680,8 @@ void rt_mutex_adjust_pi(struct task_struraw_spin_lock_irqsave (&task->pi_lock, flags); waiter = task->pi_blocked_on; -if (!waiter || waiter->list_entry.prio == task->prio) { +if (!rt_mutex_real_waiter (waiter) || + waiter->list_entry.prio == task->prio) {raw_spin_unlock_irqrestore (&task->pi_lock, flags);return ; }
@@ -1527,7 +1550,7 @@ int rt_mutex_start_proxy_lock(struct rt_ ret =task_blocks_on_rt_mutex (lock, waiter, task, detect_deadlock, flags); -if (ret && !waiter->task) { +if (ret == -EDEADLK && !waiter->task) {/* * Reset the return value. We might have * returned with -EDEADLK and the owner
kernel/rtmutex_common.h +3 -0
@@ -115,6 +115,9 @@ static inline unsigned long rt_mutex_own/* * PI-futex support (proxy locking functions, etc.): */ + +#define PI_WAKEUP_INPROGRESS ((struct rt_mutex_waiter *) 1) +extern struct task_struct *rt_mutex_next_owner (struct rt_mutex *lock);extern void rt_mutex_init_proxy_locked (struct rt_mutex *lock,struct task_struct *proxy_owner);
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI -V2
Quoted 9 lines · Thomas Gleixner
Quoted 5 lines · Thomas Gleixner
This code causes braindamage. I really wonder whether we need to remove it according to the "United Nations Convention against Torture and Other Cruel, Inhuman or Degrading Treatment or Punishment".
Ok, finally managed to twist my brain around it. Mike, can you give it a test ride ?
I'm away from box until Friday evening. I'll test, but it'll likely be ancient history by then.
-Mike
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI -V2
In reply to Thomas GleixnerQuoted 9 lines · Thomas Gleixner
Quoted 5 lines · Thomas Gleixner
This code causes braindamage. I really wonder whether we need to remove it according to the "United Nations Convention against Torture and Other Cruel, Inhuman or Degrading Treatment or Punishment".
Ok, finally managed to twist my brain around it. Mike, can you give it a test ride ?
Since Mike is out I built this version and ran it a few times. I saw a 100% reproduction rate previously. I haven't seen any errors in a handful of runs (~10) with this patch.
I do still see the hrtimer latency message on the first run, so this is likely unrelated to the issue we're addressing:
Jul 13 14:47:59 elm9m94 kernel: hrtimer: interrupt took 123924 ns
As for Thomas's changes, only a couple nitpics below:
Quoted 7 lines
kernel/futex.c +0
-3@@ -2255,18 +2265,51 @@ static int futex_wait_requeue_pi(u32 __u/* Queue the futex_q, drop the hb lock, wait for wakeup. */ futex_wait_queue_me (hb,&q, to); -spin_lock (&hb->lock); - ret =handle_early_requeue_pi_wakeup (hb,&q,&key2, to); -spin_unlock (&hb->lock);
kernel/futex.c static int futex_wait_requeue_pi(u32 __u · line 2275
Quoted 10 lines
kernel/futex.c +8
-2-if (ret) - goto out_put_keys; +/* +* Avoid races with requeue and trying to block on two mutexes +* (hb->lock and uaddr2's rtmutex) by serializing access to +* pi_blocked_on with pi_lock. +*/ +raw_spin_lock_irq (¤t->pi_lock); +if (current->pi_blocked_on) { +/* Requeue happened already */
This comment isn't quite accurate. The requeue may be in progress, which means the q.lock_ptr is not trustworthy as noted below. Consider:
/* * We have been requeued, or are in the process * of being requeued. */
kernel/futex.c static int futex_wait_requeue_pi(u32 __u · line 2280
Quoted 5 lines
kernel/futex.c +5
-0+raw_spin_unlock_irq (¤t->pi_lock); + }else { +/* +* Setting pi_blocked_on to PI_WAKEUP_INPROGRESS +* prevents a concurrent requeue from enqueuein us on
s/enqueuein/enqueueing/ not that my dictionary thinks either one of them are words ;-)
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI -V2
In reply to Thomas GleixnerQuoted 4 lines · Thomas Gleixner
On Tue, 13 Jul 2010, Thomas Gleixner wrote:
Ok, finally managed to twist my brain around it. Mike, can you give it a test ride ?
It's a bit late, but I took it out for a spin, no problem noted.
-Mike
Re: [PATCH][RT] futex: protect against pi_blocked_on corruption during requeue PI
In reply to Darren Hartkernel/futex.c retry_private: · line 1340
Quoted 10 lines · Darren Hart
kernel/futex.c +2
-0@@ -1336,6 +1336,9 @@ retry_private:requeue_pi_wake_futex (this, &key2, hb2); drop_count++;continue ; + }else if (ret == -EAGAIN ) { +/* Waiter woken by timeout or signal. */
This leaks the pi_state.
Quoted 10 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc> 4.3, else testcases will segfault due to a gcc bug.
Hi Mike,
I have seen this segfault esp with GCC v4.3.4. I am about to post this patch in ltp:
--- testcases/realtime/include/librttest.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
testcases/realtime/include/librttest.h +3 -3
@@ -118,9 +118,9 @@ static inline int atomic_add(int i, atomic_t *v)int __i; __i = i; asmvolatile ( -"lock; xaddl %0, %1;" - :"=r" (i) - :"m" (v->counter),"0" (i)); +"lock; xaddl %1, %0;" + :"=m" (v->counter) + :"r" (i),"m" (v->counter));return i + __i;#elif defined(__powerpc__)#define ISYNC_ON_SMP "\n\tisync\n"
--
Please let me know if this patch helps.
Thanks,
Gowri
Quoted 95 lines
2. apply patchlet so you can run
testcases/realtime/perf/latency/run.shat all.pthread_cond_many.c renamed from pthread_cond_many.c.org +1
-1@@ -259,7 +259,7 @@ void usage(void)int parse_args (int c,char *v) { -int handled; +int handled =1 ;switch (c) {case 'h' :usage ();3. add --realtime for no particular reason.
run.sh renamed from run.sh.org +2
-2@@ -22,7 +22,7 @@ make # process to run realtime. The remainder of the processes (if any) # will run non-realtime in any case. -nthread=5000 +nthread=500 iter=400 nproc=5@@ -39,7 +39,7 @@ i=0 i=1 while test $i -lt $nproc do - ./pthread_cond_many --broadcast -i $iter -n $nthread> $nthread.$iter.$nproc.$i.out& + ./pthread_cond_many --realtime --broadcast -i $iter -n $nthread> $nthread.$iter.$nproc.$i.out& i=`expr $i + 1` done wait4. run it.
What happens here is we hit
WARN_ON(pendowner->pi_blocked_on != waiter), this does not make it to consoles (poking sysrq-foo doesn't either). Next comesWARN_ON(!pendowner->pi_blocked_on), followed by the NULL explosion, which does make it to consoles.With explosion avoidance, I also see
pendowner->pi_blocked_on->task== NULL at times, but that, as !pendowner->pi_blocked_on, seems to be fallout. The start of bad juju is alwayspi_blocked_on!= waiter.[ 141.609268] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058 [ 141.609268] IP: [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] PGD 20e174067 PUD 20e253067 PMD 0 [ 141.609268] Oops: 0000 [#1] PREEMPT SMP [ 141.609268] last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map [ 141.609268] CPU 0 [ 141.609268] Pid: 8154, comm: pthread_cond_ma Tainted: G W 2.6.33.6-rt23 #12 MS-7502/MS-7502 [ 141.609268] RIP: 0010:[<ffffffff8106856d>] [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP: 0018:ffff88020e3cdd78 EFLAGS: 00010097 [ 141.609268] RAX: 0000000000000000 RBX: ffff8801e8eba5c0 RCX: 0000000000000000 [ 141.609268] RDX: ffff880028200000 RSI: 0000000000000046 RDI: 0000000000000009 [ 141.609268] RBP: ffff88020e3cdda8 R08: 0000000000000002 R09: 0000000000000000 [ 141.609268] R10: 0000000000000005 R11: 0000000000000000 R12: ffffffff81659068 [ 141.609268] R13: ffff8801e8ebdb58 R14: 0000000000000000 R15: ffff8801e8ebac08 [ 141.609268] FS: 00007f664d539700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000 [ 141.609268] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 141.609268] CR2: 0000000000000058 CR3: 0000000214266000 CR4: 00000000000006f0 [ 141.609268] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 141.609268] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 141.609268] Process pthread_cond_ma (pid: 8154, threadinfo ffff88020e3cc000, task ffff88020e2a4700) [ 141.609268] Stack: [ 141.609268] 0000000000000000 ffffffff81659068 0000000000000202 0000000000000000 [ 141.609268]<0> 0000000000000000 0000000080001fda ffff88020e3cddc8 ffffffff812fec48 [ 141.609268]<0> ffffffff81659068 0000000000606300 ffff88020e3cddd8 ffffffff812ff1b9 [ 141.609268] Call Trace: [ 141.609268] [<ffffffff812fec48>] rt_spin_lock_slowunlock+0x43/0x61 [ 141.609268] [<ffffffff812ff1b9>] rt_spin_unlock+0x46/0x48 [ 141.609268] [<ffffffff81067d7f>] do_futex+0x83c/0x935 [ 141.609268] [<ffffffff810c26ce>] ? handle_mm_fault+0x6de/0x6f1 [ 141.609268] [<ffffffff81067e36>] ? do_futex+0x8f3/0x935 [ 141.609268] [<ffffffff81067fba>] sys_futex+0x142/0x154 [ 141.609268] [<ffffffff81020eb0>] ? do_page_fault+0x23e/0x28e [ 141.609268] [<ffffffff81004aa7>] ? math_state_restore+0x3d/0x3f [ 141.609268] [<ffffffff81004b08>] ? do_device_not_available+0xe/0x12 [ 141.609268] [<ffffffff81002c5b>] system_call_fastpath+0x16/0x1b [ 141.609268] Code: c7 09 6d 41 81 e8 ac 34 fd ff 4c 39 ab 70 06 00 00 74 11 be 47 02 00 00 48 c7 c7 09 6d 41 81 e8 92 34 fd ff 48 8b 83 70 06 00 00<4c> 39 60 58 74 11 be 48 02 00 00 48 c7 c7 09 6d 41 81 e8 74 34 [ 141.609268] RIP [<ffffffff8106856d>] wakeup_next_waiter+0x12c/0x177 [ 141.609268] RSP<ffff88020e3cdd78> [ 141.609268] CR2: 0000000000000058 [ 141.609268] ---[ end trace 58805b944e6f93ce ]--- [ 141.609268] note: pthread_cond_ma[8154] exited with preempt_count 2(5. eyeball locks.. -> zzzzt -> report -> eyeball..)
-Mike
Quoted 41 lines · gowrishankar
Quoted 10 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of testcase bustage in earlier versions, and must be built with gcc> 4.3, else testcases will segfault due to a gcc bug.
Hi Mike,
I have seen this segfault esp with GCC v4.3.4. I am about to post this patch
in ltp:Signed-off-by Gowrishankar <[email protected]>--- testcases/realtime/include/librttest.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)testcases/realtime/include/librttest.h +3
-4@@ -118,9 +118,9 @@ static inline int atomic_add(int i, atomic_t *v)int __i; __i = i; asmvolatile ( -"lock; xaddl %0, %1;" - :"=r" (i) - :"m" (v->counter),"0" (i)); +"lock; xaddl %1, %0;" + :"=m" (v->counter) + :"r" (i),"m" (v->counter));return i + __i;#elif defined(__powerpc__)#define ISYNC_ON_SMP "\n\tisync\n" --
Quoted 1 line
Please let me know if this patch helps.
Yup, all better.
-Mike
Quoted 42 lines · Mike Galbraith
Quoted 41 lines · gowrishankar
Quoted 10 lines · Mike Galbraith
Greetings,
Stress testing, looking to trigger RCU stalls, I've managed to find a way to repeatably create fireworks. (got RCU stall, see attached)
1. download ltp-full-20100630. Needs to be this version because of
testcase bustage in earlier versions, and must be built with gcc> 4.3,
else testcases will segfault due to a gcc bug.Hi Mike,
I have seen this segfault esp with GCC v4.3.4. I am about to post this patch
in ltp:Signed-off-by Gowrishankar<[email protected]>--- testcases/realtime/include/librttest.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)testcases/realtime/include/librttest.h +3
-4@@ -118,9 +118,9 @@ static inline int atomic_add(int i, atomic_t *v)int __i; __i = i; asmvolatile ( -"lock; xaddl %0, %1;" - :"=r" (i) - :"m" (v->counter),"0" (i)); +"lock; xaddl %1, %0;" + :"=m" (v->counter) + :"r" (i),"m" (v->counter));return i + __i;#elif defined(__powerpc__)#define ISYNC_ON_SMP "\n\tisync\n" --
Quoted 3 lines
Quoted 1 line
Please let me know if this patch helps.
Yup, all better.
So with this, the "volatile struct" patch isn't necessary?
If so, perhaps we would be better off converting librttest.h to implement the atomic functions using the glibc built-ins, ie:
static inline intatomic_add(atomic_t *addr, int i)
{
return __sync_add_and_fetch(&addr->val, i);
}It's more maintainable than inline asm and better tested than a roll your own implementation. Oddly, I believe the original was patterned after what is in the Linux kernel...
--
Darren
Quoted 3 lines
-Mike
Signature · 3 lines
Darren Hart IBM Linux Technology Center Real-Time Linux Team
Quoted 5 lines · Darren Hart
Quoted 1 line · Mike Galbraith
Yup, all better.
So with this, the "volatile struct" patch isn't necessary?
Definitely no longer necessary.. on my box ;-)
-Mike