[PATCH] [MMC] Use own work queue

[PATCH] [MMC] Use own work queue

[Pierre Ossman]

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 <drzeus@drzeus.cx>
---

 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);

Re: [PATCH] [MMC] Use own work queue

[pHilipp Zabel]

Hi,

On 10/1/06, Pierre Ossman <drzeus@drzeus.cx> 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 <drzeus@drzeus.cx>

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

Re: [PATCH] [MMC] Use own work queue

[Pierre Ossman]

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

Re: [PATCH] [MMC] Use own work queue

[Russell King]

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

Re: [PATCH] [MMC] Use own work queue

[pHilipp Zabel]

On 10/13/06, Russell King <rmk+lkml@arm.linux.org.uk> 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

Re: [PATCH] [MMC] Use own work queue

[Russell King]

On Sun, Oct 15, 2006 at 11:40:31AM +0200, pHilipp Zabel wrote:
> On 10/13/06, Russell King <rmk+lkml@arm.linux.org.uk> 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

Re: [PATCH] [MMC] Use own work queue

[Pierre Ossman]

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

Re: [PATCH] [MMC] Use own work queue

[Pierre Ossman]

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