* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).