* 2.6.15-git7 oopses in ext3 during LTP runs
@ 2006-01-11 20:26 Andi Kleen
2006-01-11 20:46 ` Andrew Morton
0 siblings, 1 reply; 15+ messages in thread
From: Andi Kleen @ 2006-01-11 20:26 UTC (permalink / raw)
To: linux-kernel; +Cc: sct, akpm
Running LTP with the default runfile on a 4 virtual CPU x86-64
system gives
To reproduce: run ltp 20040908 (newer one will probably work
too) with runltp -p -q -l `uname -r` on a ext3 file system
config is x86-64 defconfig.
-Andi
----------- [cut here ] --------- [please bite here ] ---------
Kernel BUG at /home/lsrc/quilt/linux/fs/ext3/super.c:2154
invalid opcode: 0000 [1] SMP
CPU 0
Modules linked in:
Pid: 14055, comm: pdflush Not tainted 2.6.15-git7 #90
RIP: 0010:[<ffffffff801d998e>] <ffffffff801d998e>{ext3_write_super+20}
RSP: 0018:ffff810117fb1e08 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff81011f9ae800 RCX: ffffffff80492060
RDX: 0000000000000000 RSI: ffff810117fb0000 RDI: ffff81011f9ae888
RBP: ffff81011f9ae888 R08: ffff810117fb0000 R09: ffff810004f219e0
R10: 0000000000000000 R11: 0000000000000002 R12: ffff81011f9ae870
R13: ffffffff80159aee R14: ffff8100cdce1d68 R15: ffffffff80146b14
FS: 0000000000000000(0000) GS:ffffffff8060d000(0000) knlGS:0000000000000000
CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 00002aaaaab61000 CR3: 000000011baf3000 CR4: 00000000000006e0
Process pdflush (pid: 14055, threadinfo ffff810117fb0000, task ffff81011fe5c400)
Stack: ffff81011f9ae800 ffffffff8017aa43 0000000000000064 0000000000000000
ffff8100cdce1d78 ffffffff801592f9 ffff81011fe5c400 ffffffff80494000
0000000000000000 0000000000000000
Call Trace: <ffffffff8017aa43>{sync_supers+133} <ffffffff801592f9>{wb_kupdate+49}
<ffffffff80159aee>{pdflush+0} <ffffffff80159c40>{pdflush+338}
<ffffffff801592c8>{wb_kupdate+0} <ffffffff80146c7f>{kthread+203}
<ffffffff801107ca>{child_rip+8} <ffffffff80146b14>{keventd_create_kthread+0}
<ffffffff80146bb4>{kthread+0} <ffffffff801107c2>{child_rip+0}
Code: 0f 0b 68 e9 0d 43 80 c2 6a 08 c6 43 21 00 5b c3 49 c7 c0 ca
RIP <ffffffff801d998e>{ext3_write_super+20} RSP <ffff810117fb1e08>
Badness in do_exit at /home/lsrc/quilt/linux/kernel/exit.c:796
Call Trace: <ffffffff80136800>{do_exit+84} <ffffffff804024dd>{_spin_unlock_irqrestore+8}
<ffffffff80146b14>{keventd_create_kthread+0} <ffffffff801114ef>{kernel_math_error+0}
<ffffffff80159aee>{pdflush+0} <ffffffff80111c61>{do_invalid_op+163}
<ffffffff801d998e>{ext3_write_super+20} <ffffffff8040116a>{thread_return+0}
<ffffffff80110611>{error_exit+0} <ffffffff80146b14>{keventd_create_kthread+0}
<ffffffff80159aee>{pdflush+0} <ffffffff801d998e>{ext3_write_super+20}
<ffffffff801d998a>{ext3_write_super+16} <ffffffff8017aa43>{sync_supers+133}
<ffffffff801592f9>{wb_kupdate+49} <ffffffff80159aee>{pdflush+0}
<ffffffff80159c40>{pdflush+338} <ffffffff801592c8>{wb_kupdate+0}
<ffffffff80146c7f>{kthread+203} <ffffffff801107ca>{child_rip+8}
<ffffffff80146b14>{keventd_create_kthread+0} <ffffffff80146bb4>{kthread+0}
<ffffffff801107c2>{child_rip+0}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 20:26 2.6.15-git7 oopses in ext3 during LTP runs Andi Kleen @ 2006-01-11 20:46 ` Andrew Morton 2006-01-11 20:55 ` Arjan van de Ven 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2006-01-11 20:46 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, sct, Ingo Molnar Andi Kleen <ak@suse.de> wrote: > > > Running LTP with the default runfile on a 4 virtual CPU x86-64 > system gives > > To reproduce: run ltp 20040908 (newer one will probably work > too) with runltp -p -q -l `uname -r` on a ext3 file system > > config is x86-64 defconfig. > mutex_trylock() is returning the wrong value. fs/super.c:write_super() clearly took the lock. Ingo, weren't you hitting this occasionally? > > ----------- [cut here ] --------- [please bite here ] --------- > Kernel BUG at /home/lsrc/quilt/linux/fs/ext3/super.c:2154 > invalid opcode: 0000 [1] SMP > CPU 0 > Modules linked in: > Pid: 14055, comm: pdflush Not tainted 2.6.15-git7 #90 > RIP: 0010:[<ffffffff801d998e>] <ffffffff801d998e>{ext3_write_super+20} > RSP: 0018:ffff810117fb1e08 EFLAGS: 00010202 > RAX: 0000000000000001 RBX: ffff81011f9ae800 RCX: ffffffff80492060 > RDX: 0000000000000000 RSI: ffff810117fb0000 RDI: ffff81011f9ae888 > RBP: ffff81011f9ae888 R08: ffff810117fb0000 R09: ffff810004f219e0 > R10: 0000000000000000 R11: 0000000000000002 R12: ffff81011f9ae870 > R13: ffffffff80159aee R14: ffff8100cdce1d68 R15: ffffffff80146b14 > FS: 0000000000000000(0000) GS:ffffffff8060d000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b > CR2: 00002aaaaab61000 CR3: 000000011baf3000 CR4: 00000000000006e0 > Process pdflush (pid: 14055, threadinfo ffff810117fb0000, task ffff81011fe5c400) > Stack: ffff81011f9ae800 ffffffff8017aa43 0000000000000064 0000000000000000 > ffff8100cdce1d78 ffffffff801592f9 ffff81011fe5c400 ffffffff80494000 > 0000000000000000 0000000000000000 > Call Trace: <ffffffff8017aa43>{sync_supers+133} <ffffffff801592f9>{wb_kupdate+49} > <ffffffff80159aee>{pdflush+0} <ffffffff80159c40>{pdflush+338} > <ffffffff801592c8>{wb_kupdate+0} <ffffffff80146c7f>{kthread+203} > <ffffffff801107ca>{child_rip+8} <ffffffff80146b14>{keventd_create_kthread+0} > <ffffffff80146bb4>{kthread+0} <ffffffff801107c2>{child_rip+0} > > Code: 0f 0b 68 e9 0d 43 80 c2 6a 08 c6 43 21 00 5b c3 49 c7 c0 ca > RIP <ffffffff801d998e>{ext3_write_super+20} RSP <ffff810117fb1e08> > Badness in do_exit at /home/lsrc/quilt/linux/kernel/exit.c:796 > > Call Trace: <ffffffff80136800>{do_exit+84} <ffffffff804024dd>{_spin_unlock_irqrestore+8} > <ffffffff80146b14>{keventd_create_kthread+0} <ffffffff801114ef>{kernel_math_error+0} > <ffffffff80159aee>{pdflush+0} <ffffffff80111c61>{do_invalid_op+163} > <ffffffff801d998e>{ext3_write_super+20} <ffffffff8040116a>{thread_return+0} > <ffffffff80110611>{error_exit+0} <ffffffff80146b14>{keventd_create_kthread+0} > <ffffffff80159aee>{pdflush+0} <ffffffff801d998e>{ext3_write_super+20} > <ffffffff801d998a>{ext3_write_super+16} <ffffffff8017aa43>{sync_supers+133} > <ffffffff801592f9>{wb_kupdate+49} <ffffffff80159aee>{pdflush+0} > <ffffffff80159c40>{pdflush+338} <ffffffff801592c8>{wb_kupdate+0} > <ffffffff80146c7f>{kthread+203} <ffffffff801107ca>{child_rip+8} > <ffffffff80146b14>{keventd_create_kthread+0} <ffffffff80146bb4>{kthread+0} > <ffffffff801107c2>{child_rip+0} ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 20:46 ` Andrew Morton @ 2006-01-11 20:55 ` Arjan van de Ven 2006-01-11 21:07 ` Andrew Morton 2006-01-11 21:25 ` 2.6.15-git7 oopses in ext3 during LTP runs Jan Engelhardt 0 siblings, 2 replies; 15+ messages in thread From: Arjan van de Ven @ 2006-01-11 20:55 UTC (permalink / raw) To: Andrew Morton; +Cc: Andi Kleen, linux-kernel, sct, Ingo Molnar On Wed, 2006-01-11 at 12:46 -0800, Andrew Morton wrote: > Andi Kleen <ak@suse.de> wrote: > > > > > > Running LTP with the default runfile on a 4 virtual CPU x86-64 > > system gives > > > > To reproduce: run ltp 20040908 (newer one will probably work > > too) with runltp -p -q -l `uname -r` on a ext3 file system > > > > config is x86-64 defconfig. > > > > mutex_trylock() is returning the wrong value. fs/super.c:write_super() > clearly took the lock. the conversion is buggy. mutex_trylock has the same convention as spin_try_lock (which is the opposite of down_trylock). THe conversion forgot to add a ! --- linux-2.6.15/fs/ext3/super.c~ 2006-01-11 21:54:13.000000000 +0100 +++ linux-2.6.15/fs/ext3/super.c 2006-01-11 21:54:13.000000000 +0100 @@ -2150,7 +2150,7 @@ static void ext3_write_super (struct super_block * sb) { - if (mutex_trylock(&sb->s_lock) != 0) + if (!mutex_trylock(&sb->s_lock) != 0) BUG(); sb->s_dirt = 0; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 20:55 ` Arjan van de Ven @ 2006-01-11 21:07 ` Andrew Morton 2006-01-11 21:27 ` Arjan van de Ven 2006-01-11 21:25 ` 2.6.15-git7 oopses in ext3 during LTP runs Jan Engelhardt 1 sibling, 1 reply; 15+ messages in thread From: Andrew Morton @ 2006-01-11 21:07 UTC (permalink / raw) To: Arjan van de Ven; +Cc: ak, linux-kernel, sct, mingo Arjan van de Ven <arjan@infradead.org> wrote: > > On Wed, 2006-01-11 at 12:46 -0800, Andrew Morton wrote: > > Andi Kleen <ak@suse.de> wrote: > > > > > > > > > Running LTP with the default runfile on a 4 virtual CPU x86-64 > > > system gives > > > > > > To reproduce: run ltp 20040908 (newer one will probably work > > > too) with runltp -p -q -l `uname -r` on a ext3 file system > > > > > > config is x86-64 defconfig. > > > > > > > mutex_trylock() is returning the wrong value. fs/super.c:write_super() > > clearly took the lock. > > > the conversion is buggy. > > mutex_trylock has the same convention as spin_try_lock (which is the > opposite of down_trylock). THe conversion forgot to add a ! > > --- linux-2.6.15/fs/ext3/super.c~ 2006-01-11 21:54:13.000000000 +0100 > +++ linux-2.6.15/fs/ext3/super.c 2006-01-11 21:54:13.000000000 +0100 > @@ -2150,7 +2150,7 @@ > > static void ext3_write_super (struct super_block * sb) > { > - if (mutex_trylock(&sb->s_lock) != 0) > + if (!mutex_trylock(&sb->s_lock) != 0) > BUG(); > sb->s_dirt = 0; > } We expect the lock to be held on entry. Hence we expect mutex_trylock() to return zero. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 21:07 ` Andrew Morton @ 2006-01-11 21:27 ` Arjan van de Ven 2006-01-11 22:19 ` Ingo Molnar ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Arjan van de Ven @ 2006-01-11 21:27 UTC (permalink / raw) To: Andrew Morton; +Cc: ak, linux-kernel, sct, mingo > 15/fs/ext3/super.c~ 2006-01-11 21:54:13.000000000 +0100 > > +++ linux-2.6.15/fs/ext3/super.c 2006-01-11 21:54:13.000000000 +0100 > > @@ -2150,7 +2150,7 @@ > > > > static void ext3_write_super (struct super_block * sb) > > { > > - if (mutex_trylock(&sb->s_lock) != 0) > > + if (!mutex_trylock(&sb->s_lock) != 0) > > BUG(); > > sb->s_dirt = 0; > > } > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > to return zero. you are correct, and the x86-64 mutex.h is buggy --- linux-2.6.15/include/asm-x86_64/mutex.h.org 2006-01-11 22:25:37.000000000 +0100 +++ linux-2.6.15/include/asm-x86_64/mutex.h 2006-01-11 22:25:43.000000000 +0100 @@ -104,7 +104,7 @@ static inline int __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) { - if (likely(atomic_cmpxchg(count, 1, 0)) == 1) + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) return 1; else return 0; changes the asm to be the correct one for me. This is odd/evil though.. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 21:27 ` Arjan van de Ven @ 2006-01-11 22:19 ` Ingo Molnar 2006-01-11 22:40 ` Pavel Machek 2006-01-12 0:55 ` 2.6.15-git7 oopses in ext3 during LTP runs II - more problems Andi Kleen 2 siblings, 0 replies; 15+ messages in thread From: Ingo Molnar @ 2006-01-11 22:19 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andrew Morton, ak, linux-kernel, sct, Linus Torvalds * Arjan van de Ven <arjan@infradead.org> wrote: > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > to return zero. > > you are correct, and the x86-64 mutex.h is buggy ahh ... indeed! And i386 trylock was buggy too. Ingo ---- fix typo in asm-i386/mutex.h:__mutex_fastpath_trylock - noticed by Arjan van de Ven. Signed-off-by: Ingo Molnar <mingo@elte.hu> --- include/asm-i386/mutex.h.orig +++ include/asm-i386/mutex.h @@ -125,7 +125,7 @@ __mutex_fastpath_trylock(atomic_t *count * the mutex state would be. */ #ifdef __HAVE_ARCH_CMPXCHG - if (likely(atomic_cmpxchg(count, 1, 0)) == 1) + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) return 1; return 0; #else ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 21:27 ` Arjan van de Ven 2006-01-11 22:19 ` Ingo Molnar @ 2006-01-11 22:40 ` Pavel Machek 2006-01-11 22:44 ` David S. Miller 2006-01-11 22:47 ` Jesper Juhl 2006-01-12 0:55 ` 2.6.15-git7 oopses in ext3 during LTP runs II - more problems Andi Kleen 2 siblings, 2 replies; 15+ messages in thread From: Pavel Machek @ 2006-01-11 22:40 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andrew Morton, ak, linux-kernel, sct, mingo On St 11-01-06 22:27:55, Arjan van de Ven wrote: > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > to return zero. > > you are correct, and the x86-64 mutex.h is buggy > > --- linux-2.6.15/include/asm-x86_64/mutex.h.org 2006-01-11 22:25:37.000000000 +0100 > +++ linux-2.6.15/include/asm-x86_64/mutex.h 2006-01-11 22:25:43.000000000 +0100 > @@ -104,7 +104,7 @@ > static inline int > __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) > { > - if (likely(atomic_cmpxchg(count, 1, 0)) == 1) > + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) > return 1; > else > return 0; > > changes the asm to be the correct one for me. > This is odd/evil though.. likely is the evil part here. What about this? Should make this bug impossible to do.... Signed-off-by: Pavel Machek <pavel@suse.cz> diff --git a/include/linux/compiler.h b/include/linux/compiler.h --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -59,8 +59,8 @@ extern void __chk_io_ptr(void __iomem *) * specific implementations come from the above header files */ -#define likely(x) __builtin_expect(!!(x), 1) -#define unlikely(x) __builtin_expect(!!(x), 0) +#define likely(x) (__builtin_expect(!!(x), 1)) +#define unlikely(x) (__builtin_expect(!!(x), 0)) /* Optimization barrier */ #ifndef barrier diff --git a/kernel/sched.c b/kernel/sched.c --- a/kernel/sched.c +++ b/kernel/sched.c @@ -367,7 +367,7 @@ repeat_lock_task: local_irq_save(*flags); rq = task_rq(p); spin_lock(&rq->lock); - if (unlikely(rq != task_rq(p))) { + if unlikely(rq != task_rq(p)) { spin_unlock_irqrestore(&rq->lock, *flags); goto repeat_lock_task; } @@ -751,7 +751,7 @@ static int recalc_task_prio(task_t *p, u else sleep_time = (unsigned long)__sleep_time; - if (likely(sleep_time > 0)) { + if likely(sleep_time > 0) { /* * User tasks that sleep a long time are categorised as * idle and will get just interactive status to stay active & @@ -876,7 +876,7 @@ static void resched_task(task_t *p) assert_spin_locked(&task_rq(p)->lock); - if (unlikely(test_tsk_thread_flag(p, TIF_NEED_RESCHED))) + if unlikely(test_tsk_thread_flag(p, TIF_NEED_RESCHED)) return; set_tsk_thread_flag(p, TIF_NEED_RESCHED); -- Thanks, Sharp! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 22:40 ` Pavel Machek @ 2006-01-11 22:44 ` David S. Miller 2006-01-12 0:30 ` Ingo Molnar 2006-01-11 22:47 ` Jesper Juhl 1 sibling, 1 reply; 15+ messages in thread From: David S. Miller @ 2006-01-11 22:44 UTC (permalink / raw) To: pavel; +Cc: arjan, akpm, ak, linux-kernel, sct, mingo From: Pavel Machek <pavel@suse.cz> Date: Wed, 11 Jan 2006 23:40:13 +0100 > likely is the evil part here. What about this? Should make this bug > impossible to do.... > > Signed-off-by: Pavel Machek <pavel@suse.cz> This doesn't let you do: if (likely(y) || unlikely(x)) which I have had reason to use in the past. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 22:44 ` David S. Miller @ 2006-01-12 0:30 ` Ingo Molnar 0 siblings, 0 replies; 15+ messages in thread From: Ingo Molnar @ 2006-01-12 0:30 UTC (permalink / raw) To: David S. Miller; +Cc: pavel, arjan, akpm, ak, linux-kernel, sct * David S. Miller <davem@davemloft.net> wrote: > From: Pavel Machek <pavel@suse.cz> > Date: Wed, 11 Jan 2006 23:40:13 +0100 > > > likely is the evil part here. What about this? Should make this bug > > impossible to do.... > > > > Signed-off-by: Pavel Machek <pavel@suse.cz> > > This doesn't let you do: > > if (likely(y) || unlikely(x)) hm, why not? It will expand to: if ((__builtin_expect(!!(y), 1)) || (__builtin_expect(!!(x), 0))) which seems correct to me. Pavel only added extra parantheses, to make the simple case more readable. Ingo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 22:40 ` Pavel Machek 2006-01-11 22:44 ` David S. Miller @ 2006-01-11 22:47 ` Jesper Juhl 2006-01-12 8:51 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Jesper Juhl @ 2006-01-11 22:47 UTC (permalink / raw) To: Pavel Machek Cc: Arjan van de Ven, Andrew Morton, ak, linux-kernel, sct, mingo On 1/11/06, Pavel Machek <pavel@suse.cz> wrote: > On St 11-01-06 22:27:55, Arjan van de Ven wrote: > > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > > to return zero. > > > > you are correct, and the x86-64 mutex.h is buggy > > > > --- linux-2.6.15/include/asm-x86_64/mutex.h.org 2006-01-11 22:25:37.000000000 +0100 > > +++ linux-2.6.15/include/asm-x86_64/mutex.h 2006-01-11 22:25:43.000000000 +0100 > > @@ -104,7 +104,7 @@ > > static inline int > > __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) > > { > > - if (likely(atomic_cmpxchg(count, 1, 0)) == 1) > > + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) > > return 1; > > else > > return 0; > > > > changes the asm to be the correct one for me. > > This is odd/evil though.. > > likely is the evil part here. What about this? Should make this bug > impossible to do.... > > Signed-off-by: Pavel Machek <pavel@suse.cz> > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -59,8 +59,8 @@ extern void __chk_io_ptr(void __iomem *) > * specific implementations come from the above header files > */ > > -#define likely(x) __builtin_expect(!!(x), 1) > -#define unlikely(x) __builtin_expect(!!(x), 0) > +#define likely(x) (__builtin_expect(!!(x), 1)) > +#define unlikely(x) (__builtin_expect(!!(x), 0)) > > /* Optimization barrier */ > #ifndef barrier > diff --git a/kernel/sched.c b/kernel/sched.c > --- a/kernel/sched.c > +++ b/kernel/sched.c > @@ -367,7 +367,7 @@ repeat_lock_task: > local_irq_save(*flags); > rq = task_rq(p); > spin_lock(&rq->lock); > - if (unlikely(rq != task_rq(p))) { > + if unlikely(rq != task_rq(p)) { This is confusing to read. Why not keep the parenthesis around (unlikely(...)) ? Yes, it's an extra set of parenthesis that are not strictly needed now that you've added them to the likely/unlikely macros, but they don't do any harm either and make the code less surprising to read... I know that I at least think *BUG* at once when I read a line like if unlikely(rq != task_rq(p)) { and then when I find that it actually compiles fine I go dig for the reason for that, find the macro and see that all is well, but that just wasted a lot of time. -- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 22:47 ` Jesper Juhl @ 2006-01-12 8:51 ` Pavel Machek 0 siblings, 0 replies; 15+ messages in thread From: Pavel Machek @ 2006-01-12 8:51 UTC (permalink / raw) To: Jesper Juhl; +Cc: Arjan van de Ven, Andrew Morton, ak, linux-kernel, sct, mingo On St 11-01-06 23:47:53, Jesper Juhl wrote: > On 1/11/06, Pavel Machek <pavel@suse.cz> wrote: > > On St 11-01-06 22:27:55, Arjan van de Ven wrote: > > > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > > > to return zero. > > > > > > you are correct, and the x86-64 mutex.h is buggy > > > > > > --- linux-2.6.15/include/asm-x86_64/mutex.h.org 2006-01-11 22:25:37.000000000 +0100 > > > +++ linux-2.6.15/include/asm-x86_64/mutex.h 2006-01-11 22:25:43.000000000 +0100 > > > @@ -104,7 +104,7 @@ > > > static inline int > > > __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *)) > > > { > > > - if (likely(atomic_cmpxchg(count, 1, 0)) == 1) > > > + if (likely(atomic_cmpxchg(count, 1, 0) == 1)) > > > return 1; > > > else > > > return 0; > > > > > > changes the asm to be the correct one for me. > > > This is odd/evil though.. .... > > +++ b/kernel/sched.c > > @@ -367,7 +367,7 @@ repeat_lock_task: > > local_irq_save(*flags); > > rq = task_rq(p); > > spin_lock(&rq->lock); > > - if (unlikely(rq != task_rq(p))) { > > + if unlikely(rq != task_rq(p)) { > > This is confusing to read. Why not keep the parenthesis around > (unlikely(...)) ? Yes, it's an extra set of parenthesis that are not > strictly needed now that you've added them to the likely/unlikely > macros, but they don't do any harm either and make the code less > surprising to read... I know that I at least think *BUG* at once Read the email from the start, there's very nice example why the extra ()'s are evil in mutex_fast_trylock. Pavel -- Thanks, Sharp! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs II - more problems 2006-01-11 21:27 ` Arjan van de Ven 2006-01-11 22:19 ` Ingo Molnar 2006-01-11 22:40 ` Pavel Machek @ 2006-01-12 0:55 ` Andi Kleen 2006-01-12 1:14 ` Andrew Morton 2 siblings, 1 reply; 15+ messages in thread From: Andi Kleen @ 2006-01-12 0:55 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andrew Morton, linux-kernel, sct, mingo On Wednesday 11 January 2006 22:27, Arjan van de Ven wrote: > > 15/fs/ext3/super.c~ 2006-01-11 21:54:13.000000000 +0100 > > > +++ linux-2.6.15/fs/ext3/super.c 2006-01-11 21:54:13.000000000 +0100 > > > @@ -2150,7 +2150,7 @@ > > > > > > static void ext3_write_super (struct super_block * sb) > > > { > > > - if (mutex_trylock(&sb->s_lock) != 0) > > > + if (!mutex_trylock(&sb->s_lock) != 0) > > > BUG(); > > > sb->s_dirt = 0; > > > } > > > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > to return zero. > > you are correct, and the x86-64 mutex.h is buggy While this patch seemed to fix LTP my desktop running the same kernel (with mutex fix) hung the mailer while sending an unrelated mail. Again on ext3. I unfortunately don't have the backtraces anymore because I couldn't save them to disk before the reboot (and forgot to copy them to another system sorry), but they were also hanging in some JBD/ext3 functions in D, with all disk accesses hanging. So things appear to be still broken in ext3 land. -Andi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs II - more problems 2006-01-12 0:55 ` 2.6.15-git7 oopses in ext3 during LTP runs II - more problems Andi Kleen @ 2006-01-12 1:14 ` Andrew Morton 2006-01-12 1:25 ` Andi Kleen 0 siblings, 1 reply; 15+ messages in thread From: Andrew Morton @ 2006-01-12 1:14 UTC (permalink / raw) To: Andi Kleen; +Cc: arjan, linux-kernel, sct, mingo Andi Kleen <ak@suse.de> wrote: > > On Wednesday 11 January 2006 22:27, Arjan van de Ven wrote: > > > 15/fs/ext3/super.c~ 2006-01-11 21:54:13.000000000 +0100 > > > > +++ linux-2.6.15/fs/ext3/super.c 2006-01-11 21:54:13.000000000 +0100 > > > > @@ -2150,7 +2150,7 @@ > > > > > > > > static void ext3_write_super (struct super_block * sb) > > > > { > > > > - if (mutex_trylock(&sb->s_lock) != 0) > > > > + if (!mutex_trylock(&sb->s_lock) != 0) > > > > BUG(); > > > > sb->s_dirt = 0; > > > > } > > > > > > We expect the lock to be held on entry. Hence we expect mutex_trylock() > > > to return zero. > > > > you are correct, and the x86-64 mutex.h is buggy > > While this patch seemed to fix LTP my desktop running the same kernel (with > mutex fix) hung the mailer while sending an unrelated mail. Again on ext3. > > I unfortunately don't have the backtraces anymore because I couldn't > save them to disk before the reboot (and forgot to copy them > to another system sorry), but they were also hanging in some JBD/ext3 > functions in D, with all disk accesses hanging. > > So things appear to be still broken in ext3 land. Are you using MD? sata? If so, which? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs II - more problems 2006-01-12 1:14 ` Andrew Morton @ 2006-01-12 1:25 ` Andi Kleen 0 siblings, 0 replies; 15+ messages in thread From: Andi Kleen @ 2006-01-12 1:25 UTC (permalink / raw) To: Andrew Morton; +Cc: arjan, linux-kernel, sct, mingo On Thursday 12 January 2006 02:14, Andrew Morton wrote: > Are you using MD? > > sata? If so, which? None of both. old style IDE using VIA driver. -Andi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 2.6.15-git7 oopses in ext3 during LTP runs 2006-01-11 20:55 ` Arjan van de Ven 2006-01-11 21:07 ` Andrew Morton @ 2006-01-11 21:25 ` Jan Engelhardt 1 sibling, 0 replies; 15+ messages in thread From: Jan Engelhardt @ 2006-01-11 21:25 UTC (permalink / raw) To: Arjan van de Ven Cc: Andrew Morton, Andi Kleen, linux-kernel, sct, Ingo Molnar >the conversion is buggy. > >mutex_trylock has the same convention as spin_try_lock (which is the >opposite of down_trylock). THe conversion forgot to add a ! > > static void ext3_write_super (struct super_block * sb) > { >- if (mutex_trylock(&sb->s_lock) != 0) >+ if (!mutex_trylock(&sb->s_lock) != 0) How about a nicer...? if(mutex_trylock(&sb->s_lock) == 0) Jan Engelhardt -- ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2006-01-12 8:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-01-11 20:26 2.6.15-git7 oopses in ext3 during LTP runs Andi Kleen 2006-01-11 20:46 ` Andrew Morton 2006-01-11 20:55 ` Arjan van de Ven 2006-01-11 21:07 ` Andrew Morton 2006-01-11 21:27 ` Arjan van de Ven 2006-01-11 22:19 ` Ingo Molnar 2006-01-11 22:40 ` Pavel Machek 2006-01-11 22:44 ` David S. Miller 2006-01-12 0:30 ` Ingo Molnar 2006-01-11 22:47 ` Jesper Juhl 2006-01-12 8:51 ` Pavel Machek 2006-01-12 0:55 ` 2.6.15-git7 oopses in ext3 during LTP runs II - more problems Andi Kleen 2006-01-12 1:14 ` Andrew Morton 2006-01-12 1:25 ` Andi Kleen 2006-01-11 21:25 ` 2.6.15-git7 oopses in ext3 during LTP runs Jan Engelhardt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.