* Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
@ 2005-11-23 16:26 Korolev, Alexey
2005-11-23 17:13 ` Nicolas Pitre
2005-11-23 22:02 ` Nicolas Pitre
0 siblings, 2 replies; 6+ messages in thread
From: Korolev, Alexey @ 2005-11-23 16:26 UTC (permalink / raw)
To: linux-mtd
Hi All,
I faced a halting issue on multi partitioned chip when I tried to
execute simultaneous write operations.
Platform has halted, on execution of this sequence:
dd if=random of=/dev/mtd4 bs=4k count=1k&
dd if=random of=/dev/mtd5 bs=4k count=1k&
dd if=random of=/dev/mtd6 bs=4k count=1k
Halt didn't happens on two simultaneous write operations
Execution of
dd if=random of=/dev/mtd5 bs=4k count=1k&
dd if=random of=/dev/mtd6 bs=4k count=1k
was ok.
I made small investigation. Platform falls to deadlock in get_chip
function.
I was unable to definetly locate the place of the halt. But I gues it
happened here.
struct flchip_shared *shared = chip->priv;
struct flchip *contender;
spin_lock(&shared->lock);
contender = shared->writing;
if (contender && contender != chip) {
int ret = spin_trylock(contender->mutex);
spin_unlock(&shared->lock);
if (!ret)
goto retry;
spin_unlock(chip->mutex);
ret = get_chip(map, contender, contender->start, mode);
spin_lock(chip->mutex);
if (ret) {
spin_unlock(contender->mutex);
return ret;
}
timeo = jiffies + HZ;
spin_lock(&shared->lock);
}
shared->writing = chip;
if (mode == FL_ERASING)
shared->erasing = chip;
if (contender && contender != chip)
spin_unlock(contender->mutex);
spin_unlock(&shared->lock);
I slightly simplified functionality of the code and it helped, the
following code doesn't halt
struct flchip_shared *shared = chip->priv;
struct flchip *contender;
contender = shared->writing;
if (contender && contender != chip) {
yield();
timeo = jiffies + HZ;
goto retry;
}
/* We now own it */
spin_lock(&shared->lock);
shared->writing = chip;
if (mode == FL_ERASING)
shared->erasing = chip;
if (contender && contender != chip)
spin_unlock(contender->mutex);
spin_unlock(&shared->lock);
I know that it can not be solution because it reduces functionality.
I would be very much appreciate if someone advice me solution for
deadlock issue on simultaneous write operations.
Thanks,
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
2005-11-23 16:26 Deadlock in cfi_cmdset_0001.c on simultaneous write operations Korolev, Alexey
@ 2005-11-23 17:13 ` Nicolas Pitre
2005-11-23 22:02 ` Nicolas Pitre
1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Pitre @ 2005-11-23 17:13 UTC (permalink / raw)
To: Korolev, Alexey; +Cc: linux-mtd
On Wed, 23 Nov 2005, Korolev, Alexey wrote:
> Hi All,
>
> I faced a halting issue on multi partitioned chip when I tried to
> execute simultaneous write operations.
What platform are you using? SMP or not? Using CONFIG_PREEMPT or not?
> Platform has halted, on execution of this sequence:
> dd if=random of=/dev/mtd4 bs=4k count=1k&
> dd if=random of=/dev/mtd5 bs=4k count=1k&
> dd if=random of=/dev/mtd6 bs=4k count=1k
>
> Halt didn't happens on two simultaneous write operations
> Execution of
> dd if=random of=/dev/mtd5 bs=4k count=1k&
> dd if=random of=/dev/mtd6 bs=4k count=1k
> was ok.
How big are your MTD partitions? Are they located on separate
_hardware_ partitions or do some of them share one of them?
> I made small investigation. Platform falls to deadlock in get_chip
> function.
> I was unable to definetly locate the place of the halt. But I gues it
> happened here.
Could you add some printk()'s to determine if the code is looping ...
> struct flchip_shared *shared = chip->priv;
> struct flchip *contender;
> spin_lock(&shared->lock);
> contender = shared->writing;
> if (contender && contender != chip) {
> int ret = spin_trylock(contender->mutex);
> spin_unlock(&shared->lock);
> if (!ret)
> goto retry;
^^^^^^^^^^^ here?
> spin_unlock(chip->mutex);
> ret = get_chip(map, contender, contender->start, mode);
> spin_lock(chip->mutex);
> if (ret) {
> spin_unlock(contender->mutex);
> return ret;
> }
> timeo = jiffies + HZ;
> spin_lock(&shared->lock);
> }
> shared->writing = chip;
> if (mode == FL_ERASING)
> shared->erasing = chip;
> if (contender && contender != chip)
> spin_unlock(contender->mutex);
> spin_unlock(&shared->lock);
>
> I slightly simplified functionality of the code and it helped, the
> following code doesn't halt
But that code is broken.
> struct flchip_shared *shared = chip->priv;
> struct flchip *contender;
>
> contender = shared->writing;
> if (contender && contender != chip) {
> yield();
> timeo = jiffies + HZ;
> goto retry;
> }
> /* We now own it */
^^^^^^^^^^^^^^^^^^^ wrong !
Nothing prevents another thread coming along and assigning itself the
ability to write at this point, just _after_ the current thread
determined that no contender was there but _before_ the shared lock is
acquired below.
> spin_lock(&shared->lock);
> shared->writing = chip;
And then the other thread sees its ownership overwritten.
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
2005-11-23 16:26 Deadlock in cfi_cmdset_0001.c on simultaneous write operations Korolev, Alexey
2005-11-23 17:13 ` Nicolas Pitre
@ 2005-11-23 22:02 ` Nicolas Pitre
2005-11-24 15:57 ` Alexey, Korolev
1 sibling, 1 reply; 6+ messages in thread
From: Nicolas Pitre @ 2005-11-23 22:02 UTC (permalink / raw)
To: Korolev, Alexey; +Cc: linux-mtd
On Wed, 23 Nov 2005, Korolev, Alexey wrote:
> Hi All,
>
> I faced a halting issue on multi partitioned chip when I tried to
> execute simultaneous write operations.
> Platform has halted, on execution of this sequence:
> dd if=random of=/dev/mtd4 bs=4k count=1k&
> dd if=random of=/dev/mtd5 bs=4k count=1k&
> dd if=random of=/dev/mtd6 bs=4k count=1k
>
> Halt didn't happens on two simultaneous write operations
> Execution of
> dd if=random of=/dev/mtd5 bs=4k count=1k&
> dd if=random of=/dev/mtd6 bs=4k count=1k
> was ok.
I tested the above and cannot reproduce any hang whatsoever, even with
additional parallel writes.
> I made small investigation. Platform falls to deadlock in get_chip
> function.
> I was unable to definetly locate the place of the halt. But I gues it
> happened here.
>
> struct flchip_shared *shared = chip->priv;
> struct flchip *contender;
> spin_lock(&shared->lock);
> contender = shared->writing;
> if (contender && contender != chip) {
> int ret = spin_trylock(contender->mutex);
> spin_unlock(&shared->lock);
> if (!ret)
> goto retry;
> spin_unlock(chip->mutex);
> ret = get_chip(map, contender, contender->start, mode);
> spin_lock(chip->mutex);
> if (ret) {
> spin_unlock(contender->mutex);
> return ret;
> }
> timeo = jiffies + HZ;
> spin_lock(&shared->lock);
> }
> shared->writing = chip;
> if (mode == FL_ERASING)
> shared->erasing = chip;
> if (contender && contender != chip)
> spin_unlock(contender->mutex);
> spin_unlock(&shared->lock);
Actually, the above is a bit too strict. I don't know if the following
patch will fix your problem, but it certainly makes it look more correct
from a theoretical point of view.
diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 143f01a..a4b07e2 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -644,9 +644,8 @@ static int get_chip(struct map_info *map
*
* - contension arbitration is handled in the owner's context.
*
- * The 'shared' struct can be read when its lock is taken.
- * However any writes to it can only be made when the current
- * owner's lock is also held.
+ * The 'shared' struct can be read and/or written only when
+ * its lock is taken.
*/
struct flchip_shared *shared = chip->priv;
struct flchip *contender;
@@ -675,14 +674,13 @@ static int get_chip(struct map_info *map
}
timeo = jiffies + HZ;
spin_lock(&shared->lock);
+ spin_unlock(contender->mutex);
}
/* We now own it */
shared->writing = chip;
if (mode == FL_ERASING)
shared->erasing = chip;
- if (contender && contender != chip)
- spin_unlock(contender->mutex);
spin_unlock(&shared->lock);
}
Nicolas
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
2005-11-23 22:02 ` Nicolas Pitre
@ 2005-11-24 15:57 ` Alexey, Korolev
2005-11-24 16:39 ` Nicolas Pitre
0 siblings, 1 reply; 6+ messages in thread
From: Alexey, Korolev @ 2005-11-24 15:57 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: linux-mtd
Nicolas,
I'm using non SMP platform ( Mainstone II). CONFIG_PREEMPT is disabled.
Partition size is 8MB. Current configuration: each logical volume is
located on each h/w partition. Logical volumes don't share h/w partitions.
I also disabled erase suspend on write feature.
I applied code which you have send in previous letter.
>diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c
b/drivers/mtd/chips/cfi_cmdset_0001.c
>index 143f01a..a4b07e2 100644
>--- a/drivers/mtd/chips/cfi_cmdset_0001.c
>+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
>@@ -644,9 +644,8 @@ static int get_chip(struct map_info *map
> *
> * - contension arbitration is handled in the owner's
context.
> *
>- * The 'shared' struct can be read when its lock is
taken.
>- * However any writes to it can only be made when the
current
>- * owner's lock is also held.
>+ * The 'shared' struct can be read and/or written only
when
>+ * its lock is taken.
> */
> struct flchip_shared *shared = chip->priv;
> struct flchip *contender;
>@@ -675,14 +674,13 @@ static int get_chip(struct map_info *map
> }
> timeo = jiffies + HZ;
> spin_lock(&shared->lock);
>+ spin_unlock(contender->mutex);
> }
>
> /* We now own it */
> shared->writing = chip;
> if (mode == FL_ERASING)
> shared->erasing = chip;
>- if (contender && contender != chip)
>- spin_unlock(contender->mutex);
> spin_unlock(&shared->lock);
> }
After that code behavior has changed.
It didn't halt on basic simultaneous write operations.
But it failed to kernel panic in our test case. (Five applications, each
of them performs writing, erasing and reading own logical volume )
Here is kernel panic message:
After this message I received two more almost the same as this kernel
panic messages.
Unable to handle
kernel NULL pointer dereference at virtual address 00000000
pgd = c34d0000
[00000000] *pgd=a38ca031, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1]
Modules linked in:
CPU: 0
PC is at dequeue_task+0x10/0x7c
LR is at deactivate_task+0x24/0x30
pc : [<c0030eb8>] lr : [<c003129c>] Tainted: P
sp : c391dfa8 ip : 00000000 fp : c391dfb4
r10: c3982300 r9 : c02095a8 r8 : c3c732d4
r7 : 00000007 r6 : c02deba0 r5 : c013b990 r4 : c3982300
r3 : 00000008 r2 : 00000000 r1 : 00000000 r0 : c3982300
Flags: Nzcv IRQs off FIQs on Mode SVC_32 Segment user
Control: 397F Table: A3564000 DAC: 00000015
Process testapp_fm1 (pid: 939, stack limit = 0xc391c1a4)
Stack: (0xc391dfa8 to 0xc391e000)
dfa0: c391dfc8 c391dfb8 c003129c c0030eb4 02c76300
c391e004
dfc0: c391dfcc c01a0928 c0031284 02734e47 33c93d00 00000075 c3982450
c3c732f0
dfe0: c391e08c c02deba0 00000007 c3c732d4 00000001 00000001 c391e0c8
c391e008
Backtrace:
[<c0030ea8>] (dequeue_task+0x0/0x7c) from [<c003129c>]
(deactivate_task+0x24/0x3
0)
[<c0031278>] (deactivate_task+0x0/0x30) from [<c01a0928>]
(schedule+0x1a8/0x4c8)
r4 = 02C76300
[<c01a0780>] (schedule+0x0/0x4c8) from [<c013b994>] (get_chip+0xb80/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013b0b0>] (get_chip+0x29c/0xbb8)
[<c013ae14>] (get_chip+0x0/0xbb8) from [<c013d3dc>]
(do_write_buffer+0x188/0x194
0)
[<c013d254>] (do_write_buffer+0x0/0x1940) from [<c013ece0>]
(cfi_intelext_write_
buffers+0x14c/0x1ac)
[<c013eb94>] (cfi_intelext_write_buffers+0x0/0x1ac) from [<c012e794>]
(part_writ
e+0xc0/0x108)
[<c012e6d4>] (part_write+0x0/0x108)
Code: e1a0c00d e92dd800 e24cb004 e1a0c001 (e5913000)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
2005-11-24 15:57 ` Alexey, Korolev
@ 2005-11-24 16:39 ` Nicolas Pitre
2005-11-25 13:27 ` Alexey, Korolev
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Pitre @ 2005-11-24 16:39 UTC (permalink / raw)
To: Alexey, Korolev; +Cc: linux-mtd
On Thu, 24 Nov 2005, Alexey, Korolev wrote:
> Nicolas,
>
> I'm using non SMP platform ( Mainstone II). CONFIG_PREEMPT is disabled.
What kernel version are you using?
Can you send me your kernel .config? I'll try to reproduce it here.
> Partition size is 8MB. Current configuration: each logical volume is located
> on each h/w partition. Logical volumes don't share h/w partitions.
This is Sibley flash?
> I also disabled erase suspend on write feature.
Why?
> I applied code which you have send in previous letter.
> After that code behavior has changed.
> It didn't halt on basic simultaneous write operations.
Actually, I wonder why. Especially with CONFIG_PREEMPT on non SMP
system all spin_locks are just no ops.
> But it failed to kernel panic in our test case. (Five applications, each of
> them performs writing, erasing and reading own logical volume )
Can you share your test application with me?
> Here is kernel panic message:
> After this message I received two more almost the same as this kernel panic
> messages.
>
[...]
> Stack: (0xc391dfa8 to 0xc391e000)
> dfa0: c391dfc8 c391dfb8 c003129c c0030eb4 02c76300 c391e004
> dfc0: c391dfcc c01a0928 c0031284 02734e47 33c93d00 00000075 c3982450 c3c732f0
> dfe0: c391e08c c02deba0 00000007 c3c732d4 00000001 00000001 c391e0c8 c391e008
> Backtrace:
[...]
This looks extremely suspicious, given that the backtrace has at least
40 calls and the stack cannot contain all of them given its location
(the kernel stack is 8kb aligned). So this really looks like a kernel
stack overflow, and frankly I wonder how you managed that.
Did you modify your kernel somehow? What patches if any did you apply
to it?
Nicolas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Deadlock in cfi_cmdset_0001.c on simultaneous write operations.
2005-11-24 16:39 ` Nicolas Pitre
@ 2005-11-25 13:27 ` Alexey, Korolev
0 siblings, 0 replies; 6+ messages in thread
From: Alexey, Korolev @ 2005-11-25 13:27 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: linux-mtd
Nicolas Pitre wrote:
> On Thu, 24 Nov 2005, Alexey, Korolev wrote:
>
> > Nicolas,
> >
> > I'm using non SMP platform ( Mainstone II). CONFIG_PREEMPT is disabled.
>
> What kernel version are you using?
>
linux 2.6.11
> Can you send me your kernel .config? I'll try to reproduce it here.
>
> > Partition size is 8MB. Current configuration: each logical volume is
> located
> > on each h/w partition. Logical volumes don't share h/w partitions.
>
> This is Sibley flash?
>
Yes it is M18 flash chip.
> > I also disabled erase suspend on write feature.
>
> Why?
>
I thought that it would be better for the bug localization. Please
correct me if I'm wrong. The code recursion in get_chip function is
mostly related to usage of erase suspend on write feature.
Code just fall to sleep on attempt to get busy chip if I disable erase
suspend on write. It just showed to me that it is not a problem with
erase suspend.
> > I applied code which you have send in previous letter.
> > After that code behavior has changed.
> > It didn't halt on basic simultaneous write operations.
>
> Actually, I wonder why. Especially with CONFIG_PREEMPT on non SMP
> system all spin_locks are just no ops.
>
> > But it failed to kernel panic in our test case. (Five applications,
> each of
> > them performs writing, erasing and reading own logical volume )
>
> Can you share your test application with me?
>
The test application is a part of rather big test harness.
I'm will try to find a way for you to reproduce the issue.
> > Here is kernel panic message:
> > After this message I received two more almost the same as this
> kernel panic
> > messages.
> >
> [...]
> > Stack: (0xc391dfa8 to 0xc391e000)
> > dfa0: c391dfc8 c391dfb8 c003129c c0030eb4 02c76300
> c391e004
> > dfc0: c391dfcc c01a0928 c0031284 02734e47 33c93d00 00000075 c3982450
> c3c732f0
> > dfe0: c391e08c c02deba0 00000007 c3c732d4 00000001 00000001 c391e0c8
> c391e008
> > Backtrace:
> [...]
>
> This looks extremely suspicious, given that the backtrace has at least
> 40 calls and the stack cannot contain all of them given its location
> (the kernel stack is 8kb aligned). So this really looks like a kernel
> stack overflow, and frankly I wonder how you managed that.
>
> Did you modify your kernel somehow? What patches if any did you apply
> to it?
>
Yes we modified kernel. We made own patches for kernel. But it doesn't
relate to chip getting process.
I think it will be possible to reproduce the issue on default
configuration . I need some time to find a way how to do it.
Thanks,
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-11-25 13:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-23 16:26 Deadlock in cfi_cmdset_0001.c on simultaneous write operations Korolev, Alexey
2005-11-23 17:13 ` Nicolas Pitre
2005-11-23 22:02 ` Nicolas Pitre
2005-11-24 15:57 ` Alexey, Korolev
2005-11-24 16:39 ` Nicolas Pitre
2005-11-25 13:27 ` Alexey, Korolev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox