2006-10-01 12:42:58

by Pierre Ossman

[permalink] [raw]
Subject: [PATCH] [MMC] Use own work queue

The MMC layer uses the standard work queue for doing card detection. As
this queue is shared with other crucial subsystems, the effects of a long
(and perhaps buggy) detection can cause the system to be unusable. E.g. the
keyboard stops working while the detection routine is running.

The solution is to add a specific mmc work queue to run the detection code
in. This is similar to how other subsystems handle detection (a full
kernel thread is the most common theme).

Signed-off-by: Pierre Ossman <[email protected]>
---

drivers/mmc/mmc.c | 6 +++---
drivers/mmc/mmc.h | 4 ++++
drivers/mmc/mmc_sysfs.c | 35 ++++++++++++++++++++++++++++++++++-
3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 5b9caa7..ee8863c 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1166,9 +1166,9 @@ static void mmc_setup(struct mmc_host *h
void mmc_detect_change(struct mmc_host *host, unsigned long delay)
{
if (delay)
- schedule_delayed_work(&host->detect, delay);
+ mmc_schedule_delayed_work(&host->detect, delay);
else
- schedule_work(&host->detect);
+ mmc_schedule_work(&host->detect);
}

EXPORT_SYMBOL(mmc_detect_change);
@@ -1311,7 +1311,7 @@ EXPORT_SYMBOL(mmc_remove_host);
*/
void mmc_free_host(struct mmc_host *host)
{
- flush_scheduled_work();
+ mmc_flush_scheduled_work();
mmc_free_host_sysfs(host);
}

diff --git a/drivers/mmc/mmc.h b/drivers/mmc/mmc.h
index 97bae00..cd5e0ab 100644
--- a/drivers/mmc/mmc.h
+++ b/drivers/mmc/mmc.h
@@ -18,4 +18,8 @@ struct mmc_host *mmc_alloc_host_sysfs(in
int mmc_add_host_sysfs(struct mmc_host *host);
void mmc_remove_host_sysfs(struct mmc_host *host);
void mmc_free_host_sysfs(struct mmc_host *host);
+
+int mmc_schedule_work(struct work_struct *work);
+int mmc_schedule_delayed_work(struct work_struct *work, unsigned long delay);
+void mmc_flush_scheduled_work(void);
#endif
diff --git a/drivers/mmc/mmc_sysfs.c b/drivers/mmc/mmc_sysfs.c
index a2a35fd..10cc973 100644
--- a/drivers/mmc/mmc_sysfs.c
+++ b/drivers/mmc/mmc_sysfs.c
@@ -13,6 +13,7 @@ #include <linux/module.h>
#include <linux/init.h>
#include <linux/device.h>
#include <linux/idr.h>
+#include <linux/workqueue.h>

#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -317,10 +318,41 @@ void mmc_free_host_sysfs(struct mmc_host
class_device_put(&host->class_dev);
}

+static struct workqueue_struct *workqueue;
+
+/*
+ * Internal function. Schedule work in the MMC work queue.
+ */
+int mmc_schedule_work(struct work_struct *work)
+{
+ return queue_work(workqueue, work);
+}
+
+/*
+ * Internal function. Schedule delayed work in the MMC work queue.
+ */
+int mmc_schedule_delayed_work(struct work_struct *work, unsigned long delay)
+{
+ return queue_delayed_work(workqueue, work, delay);
+}
+
+/*
+ * Internal function. Flush all scheduled work from the MMC work queue.
+ */
+void mmc_flush_scheduled_work(void)
+{
+ flush_workqueue(workqueue);
+}

static int __init mmc_init(void)
{
- int ret = bus_register(&mmc_bus_type);
+ int ret;
+
+ workqueue = create_singlethread_workqueue("kmmcd");
+ if (!workqueue)
+ return -ENOMEM;
+
+ ret = bus_register(&mmc_bus_type);
if (ret == 0) {
ret = class_register(&mmc_host_class);
if (ret)
@@ -333,6 +365,7 @@ static void __exit mmc_exit(void)
{
class_unregister(&mmc_host_class);
bus_unregister(&mmc_bus_type);
+ destroy_workqueue(workqueue);
}

module_init(mmc_init);


2006-10-07 14:17:12

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

Hi,

On 10/1/06, Pierre Ossman <[email protected]> wrote:
> The MMC layer uses the standard work queue for doing card detection. As
> this queue is shared with other crucial subsystems, the effects of a long
> (and perhaps buggy) detection can cause the system to be unusable. E.g. the
> keyboard stops working while the detection routine is running.
>
> The solution is to add a specific mmc work queue to run the detection code
> in. This is similar to how other subsystems handle detection (a full
> kernel thread is the most common theme).
>
> Signed-off-by: Pierre Ossman <[email protected]>

This patch makes pxamci stop working for me on a HTC Magician (PXA272).
Switching from 2.6.18 to 2.6.19-rc1 I got a kernel panic:

mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
PXAMCI: clkrt = 0 cmdat = 0
VFS: Cannot open root device "mmcblk0p2" or unknown-block(0,0)
Please append a correct "root=" boot option
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)

After removing this patch from 2.6.19-rc1, everything is working again.
Are there any changes to pxamci.c needed to be compatible with it?

regards
Philipp

2006-10-09 21:04:51

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

pHilipp Zabel wrote:
>
> This patch makes pxamci stop working for me on a HTC Magician (PXA272).
> Switching from 2.6.18 to 2.6.19-rc1 I got a kernel panic:
>
> mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
> PXAMCI: clkrt = 0 cmdat = 0
> VFS: Cannot open root device "mmcblk0p2" or unknown-block(0,0)
> Please append a correct "root=" boot option
> Kernel panic - not syncing: VFS: Unable to mount root fs on
> unknown-block(0,0)
>
> After removing this patch from 2.6.19-rc1, everything is working again.
> Are there any changes to pxamci.c needed to be compatible with it?
>

No, the drivers shouldn't be affected. As this is a root device, my
guess would be that you have a race in your bootup that is causing problem.

Are you using initrd for this device? And could you get a complete dmesg
dump?

Rgds
Pierre

2006-10-13 07:56:40

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

On Mon, Oct 09, 2006 at 11:04:59PM +0200, Pierre Ossman wrote:
> pHilipp Zabel wrote:
> >
> > This patch makes pxamci stop working for me on a HTC Magician (PXA272).
> > Switching from 2.6.18 to 2.6.19-rc1 I got a kernel panic:
> >
> > mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
> > PXAMCI: clkrt = 0 cmdat = 0
> > VFS: Cannot open root device "mmcblk0p2" or unknown-block(0,0)
> > Please append a correct "root=" boot option
> > Kernel panic - not syncing: VFS: Unable to mount root fs on
> > unknown-block(0,0)
> >
> > After removing this patch from 2.6.19-rc1, everything is working again.
> > Are there any changes to pxamci.c needed to be compatible with it?
> >
>
> No, the drivers shouldn't be affected. As this is a root device, my
> guess would be that you have a race in your bootup that is causing problem.

The problem is likely that the boot is continuing in parallel with
detecting the card, because the card detection is running in its own
separate thread. Meanwhile, the init thread is trying to read from
the as-yet missing root device and erroring out.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-10-15 09:40:34

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

On 10/13/06, Russell King <[email protected]> wrote:
> On Mon, Oct 09, 2006 at 11:04:59PM +0200, Pierre Ossman wrote:
> > pHilipp Zabel wrote:
> > >
> > > This patch makes pxamci stop working for me on a HTC Magician (PXA272).
> > > Switching from 2.6.18 to 2.6.19-rc1 I got a kernel panic:
> > >
> > > mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
> > > PXAMCI: clkrt = 0 cmdat = 0
> > > VFS: Cannot open root device "mmcblk0p2" or unknown-block(0,0)
> > > Please append a correct "root=" boot option
> > > Kernel panic - not syncing: VFS: Unable to mount root fs on
> > > unknown-block(0,0)
> > >
> > > After removing this patch from 2.6.19-rc1, everything is working again.
> > > Are there any changes to pxamci.c needed to be compatible with it?
> > >
> >
> > No, the drivers shouldn't be affected. As this is a root device, my
> > guess would be that you have a race in your bootup that is causing problem.
>
> The problem is likely that the boot is continuing in parallel with
> detecting the card, because the card detection is running in its own
> separate thread. Meanwhile, the init thread is trying to read from
> the as-yet missing root device and erroring out.

Thanks, I can work around this by using the rootdelay kernel parameter.
So does that mean this is the expected behavior, or should I do anything
in the bootup sequence to make the init process wait for mmc detection?

regards
Philipp

2006-10-15 15:39:00

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

On Sun, Oct 15, 2006 at 11:40:31AM +0200, pHilipp Zabel wrote:
> On 10/13/06, Russell King <[email protected]> wrote:
> >The problem is likely that the boot is continuing in parallel with
> >detecting the card, because the card detection is running in its own
> >separate thread. Meanwhile, the init thread is trying to read from
> >the as-yet missing root device and erroring out.
>
> Thanks, I can work around this by using the rootdelay kernel parameter.
> So does that mean this is the expected behavior, or should I do anything
> in the bootup sequence to make the init process wait for mmc detection?

That's up to Pierre now. I originally ensured that the initial boot
time card detection was synchronous with the boot to avoid unexpected
problems like this.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-10-15 19:58:43

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

Russell King wrote:
> The problem is likely that the boot is continuing in parallel with
> detecting the card, because the card detection is running in its own
> separate thread. Meanwhile, the init thread is trying to read from
> the as-yet missing root device and erroring out.
>
>

That's what I suspect as well. I know using a root device on USB has
these kinds of problems. And the solution I've mostly seen is adding a
delay somewhere in initrd.

My experience with embedded systems is limited unfortunately, Perhaps
Russell has some nice solution for Philipp? :)

Rgds

--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
OLPC, developer http://www.laptop.org

2006-10-22 09:22:38

by Pierre Ossman

[permalink] [raw]
Subject: Re: [PATCH] [MMC] Use own work queue

pHilipp Zabel wrote:
>
> Thanks, I can work around this by using the rootdelay kernel parameter.
> So does that mean this is the expected behavior, or should I do anything
> in the bootup sequence to make the init process wait for mmc detection?
>

USB, which has an almost identical problem, usually uses a "sleep" in
initrd.

If you want to be a bit more fancy, you could try to listen to hotplug
events and look for the kernel creating the relevant block device.

Rgds
--
-- Pierre Ossman

Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org