* 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 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
* 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: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: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 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 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
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).