2009-07-15 11:52:39

by Tobias Diedrich

[permalink] [raw]
Subject: block2mtd and ubi are initialized too early when compiled in on 2.6.31-rc2

On 2.6.31-rc2 the block2mtd drivers module_init is called before any
block devices have been registered.
Also ubi is initialized pretty early and fails completely if an mtd
specified on the command line could not be found.
IMO ubi should at least complete initialization so that attaching
the mtd later with ubiattach would still work.
I'm working around this two hacky patches that create a kernel
thread and retry every second for 20 seconds when the first try
doesn't work.
(Obviously this means rootdelay=$somenumber is needed)
I tried using the async infrastructure, but apparently
async_synchronize_full is called somewhere between registering the
async probe thread and the target USB device being registered by the
USB subsystem, which halts boot until my 20 second timeout, and the
USB stick is only detected afterwards.

FWIW I want to use a erasesize aware FS on my USB stick (whose
builtin FTL has abysmal write performance if writes are less than
the erasesize) and also be able to use this as my root fs.
So my setup is usb_storage->block2mtd->ubi->ubifs

Index: linux-2.6.31-rc2/drivers/mtd/devices/block2mtd.c
===================================================================
--- linux-2.6.31-rc2.orig/drivers/mtd/devices/block2mtd.c 2009-07-11 22:22:34.318636261 +0200
+++ linux-2.6.31-rc2/drivers/mtd/devices/block2mtd.c 2009-07-12 00:05:45.937512203 +0200
@@ -17,6 +17,8 @@
#include <linux/buffer_head.h>
#include <linux/mutex.h>
#include <linux/mount.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>

#define ERROR(fmt, args...) printk(KERN_ERR "block2mtd: " fmt "\n" , ## args)
#define INFO(fmt, args...) printk(KERN_INFO "block2mtd: " fmt "\n" , ## args)
@@ -373,8 +375,30 @@
static char block2mtd_paramline[80 + 12]; /* 80 for device, 12 for erase size */
#endif

+struct block2mtd_setupasync_params {
+ char *name;
+ int erase_size;
+};
+
+static int block2mtd_setupasync(void *p)
+{
+ struct block2mtd_setupasync_params *params = p;
+ int i;
+
+ printk(KERN_WARNING "block2mtd: spawned kernel thread for async waiting on '%s'\n", params->name);
+ for (i=0; i<20; i++) {
+ msleep(1000);
+
+ if (add_device(params->name, params->erase_size) != NULL)
+ break;
+ }
+ kfree(params->name);
+ kfree(params);
+
+ return 0;
+}

-static int block2mtd_setup2(const char *val)
+static int block2mtd_setup2(const char *val, int async)
{
char buf[80 + 12]; /* 80 for device, 12 for erase size */
char *str = buf;
@@ -382,6 +406,7 @@
char *name;
size_t erase_size = PAGE_SIZE;
int i, ret;
+ struct block2mtd_setupasync_params *params;

if (strnlen(val, sizeof(buf)) >= sizeof(buf))
parse_err("parameter too long");
@@ -409,16 +434,32 @@
}
}

- add_device(name, erase_size);
+ if (add_device(name, erase_size) != NULL)
+ return 0;
+
+ params = kzalloc(sizeof(struct block2mtd_setupasync_params), GFP_KERNEL);
+ if (!params)
+ return 0;
+
+ params->name = kmalloc(strlen(name)+1, GFP_KERNEL);
+ params->erase_size = erase_size;
+ if (!params->name) {
+ kfree(params);
+ return 0;
+ }
+
+ memcpy(params->name, name, strlen(name)+1);
+
+ if (async)
+ kthread_run(block2mtd_setupasync, params, "block2mtd/setupasync");

return 0;
}

-
static int block2mtd_setup(const char *val, struct kernel_param *kp)
{
#ifdef MODULE
- return block2mtd_setup2(val);
+ return block2mtd_setup2(val, 0);
#else
/* If more parameters are later passed in via
/sys/module/block2mtd/parameters/block2mtd
@@ -426,7 +467,7 @@
we can parse the argument now. */

if (block2mtd_init_called)
- return block2mtd_setup2(val);
+ return block2mtd_setup2(val, 0);

/* During early boot stage, we only save the parameters
here. We must parse them later: if the param passed
@@ -451,7 +492,7 @@

#ifndef MODULE
if (strlen(block2mtd_paramline))
- ret = block2mtd_setup2(block2mtd_paramline);
+ ret = block2mtd_setup2(block2mtd_paramline, 1);
block2mtd_init_called = 1;
#endif

Index: linux-2.6.31-rc2/drivers/mtd/ubi/build.c
===================================================================
--- linux-2.6.31-rc2.orig/drivers/mtd/ubi/build.c 2009-07-11 23:18:58.839398443 +0200
+++ linux-2.6.31-rc2/drivers/mtd/ubi/build.c 2009-07-12 00:04:28.721126461 +0200
@@ -42,6 +42,7 @@
#include <linux/log2.h>
#include <linux/kthread.h>
#include <linux/reboot.h>
+#include <linux/delay.h>
#include "ubi.h"

/* Maximum length of the 'mtd=' parameter */
@@ -1136,6 +1137,40 @@
return mtd;
}

+static int ubi_init_attach_mtd(void *data)
+{
+ struct mtd_dev_param *p = data;
+ struct mtd_info *mtd;
+ int err;
+ int retries = 200;
+
+ printk(KERN_WARNING "ubi_init_attach_mtd(%s): waiting async for device to appear\n", p->name);
+ do {
+ mtd = open_mtd_device(p->name);
+ if (IS_ERR(mtd)) {
+ if (retries-- > 0) {
+ msleep(100);
+ continue;
+ }
+ err = PTR_ERR(mtd);
+ ubi_err("cannot attach mtd %s: %d\n", p->name, err);
+ return 0;
+ }
+ break;
+ } while (1);
+
+ mutex_lock(&ubi_devices_mutex);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
+ p->vid_hdr_offs);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ put_mtd_device(mtd);
+ ubi_err("cannot attach mtd%d: %d", mtd->index, err);
+ }
+
+ return 0;
+}
+
static int __init ubi_init(void)
{
int err, i, k;
@@ -1176,28 +1211,8 @@
goto out_dev_unreg;

/* Attach MTD devices */
- for (i = 0; i < mtd_devs; i++) {
- struct mtd_dev_param *p = &mtd_dev_param[i];
- struct mtd_info *mtd;
-
- cond_resched();
-
- mtd = open_mtd_device(p->name);
- if (IS_ERR(mtd)) {
- err = PTR_ERR(mtd);
- goto out_detach;
- }
-
- mutex_lock(&ubi_devices_mutex);
- err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
- p->vid_hdr_offs);
- mutex_unlock(&ubi_devices_mutex);
- if (err < 0) {
- put_mtd_device(mtd);
- ubi_err("cannot attach mtd%d", mtd->index);
- goto out_detach;
- }
- }
+ for (i = 0; i < mtd_devs; i++)
+ kthread_run(ubi_init_attach_mtd, &mtd_dev_param[i], "ubi/mtdprobe%d", i);

return 0;


--
Tobias PGP: http://9ac7e0bc.uguu.de


2009-07-16 12:21:24

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: block2mtd and ubi are initialized too early when compiled in on 2.6.31-rc2

Hi,

On Wed, 2009-07-15 at 13:52 +0200, Tobias Diedrich wrote:
> On 2.6.31-rc2 the block2mtd drivers module_init is called before any
> block devices have been registered.

Hmm, ok. Is this because block devices are registered asynchronously?
Could you please point to the code where it is done, just for reference.

> Also ubi is initialized pretty early and fails completely if an mtd
> specified on the command line could not be found.

Hmm...

> IMO ubi should at least complete initialization so that attaching
> the mtd later with ubiattach would still work.
> I'm working around this two hacky patches that create a kernel
> thread and retry every second for 20 seconds when the first try
> doesn't work.
> (Obviously this means rootdelay=$somenumber is needed)
> I tried using the async infrastructure, but apparently
> async_synchronize_full is called somewhere between registering the
> async probe thread and the target USB device being registered by the
> USB subsystem, which halts boot until my 20 second timeout, and the
> USB stick is only detected afterwards.
>
> FWIW I want to use a erasesize aware FS on my USB stick (whose
> builtin FTL has abysmal write performance if writes are less than
> the erasesize) and also be able to use this as my root fs.
> So my setup is usb_storage->block2mtd->ubi->ubifs

Hmm, how other subsystems solve this problem? Any pointer to the code?

--
Best regards,
Artem Bityutskiy (Битюцкий Артём)

2009-07-16 14:28:21

by Tobias Diedrich

[permalink] [raw]
Subject: Re: block2mtd and ubi are initialized too early when compiled in on 2.6.31-rc2

Artem Bityutskiy wrote:
> Hi,
>
> On Wed, 2009-07-15 at 13:52 +0200, Tobias Diedrich wrote:
> > On 2.6.31-rc2 the block2mtd drivers module_init is called before any
> > block devices have been registered.
>
> Hmm, ok. Is this because block devices are registered asynchronously?
> Could you please point to the code where it is done, just for reference.

AFAICS yes.
All module_init()s are called asynchronously from init/main.c:do_initcalls()

> > Also ubi is initialized pretty early and fails completely if an mtd
> > specified on the command line could not be found.
>
> Hmm...

> > IMO ubi should at least complete initialization so that attaching
> > the mtd later with ubiattach would still work.
> > I'm working around this two hacky patches that create a kernel
> > thread and retry every second for 20 seconds when the first try
> > doesn't work.
> > (Obviously this means rootdelay=$somenumber is needed)
> > I tried using the async infrastructure, but apparently

> > async_synchronize_full is called somewhere between registering the
> > async probe thread and the target USB device being registered by the
> > USB subsystem, which halts boot until my 20 second timeout, and the
> > USB stick is only detected afterwards.
> >
> > FWIW I want to use a erasesize aware FS on my USB stick (whose
> > builtin FTL has abysmal write performance if writes are less than
> > the erasesize) and also be able to use this as my root fs.
> > So my setup is usb_storage->block2mtd->ubi->ubifs
>
> Hmm, how other subsystems solve this problem? Any pointer to the code?

At least for mtd I've found there is a hooking mechanism:
You can use register_mtd_user() to register a callback which gets
called for each new mtd device.
I've modified my workaround patch to use that instead of a polling
thread.

AFAICS there is nothing directly comparable for block devices.
However there is drivers/base/dd.c:wait_for_device_probe() which is
used by init/do_mounts.c and also by drivers/scsi/scsi_wait_scan.c
to wait for device probing to finish.
I did not get around to try using it though.

Index: linux-2.6.31-rc3/drivers/mtd/devices/block2mtd.c
===================================================================
--- linux-2.6.31-rc3.orig/drivers/mtd/devices/block2mtd.c 2009-07-15 14:35:22.184277464 +0200
+++ linux-2.6.31-rc3/drivers/mtd/devices/block2mtd.c 2009-07-16 00:01:57.905961095 +0200
@@ -17,6 +17,9 @@
#include <linux/buffer_head.h>
#include <linux/mutex.h>
#include <linux/mount.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/async.h>

#define ERROR(fmt, args...) printk(KERN_ERR "block2mtd: " fmt "\n" , ## args)
#define INFO(fmt, args...) printk(KERN_INFO "block2mtd: " fmt "\n" , ## args)
@@ -373,8 +376,51 @@
static char block2mtd_paramline[80 + 12]; /* 80 for device, 12 for erase size */
#endif

+struct block2mtd_setupasync_params {
+ char *name;
+ int erase_size;
+ int attached;
+ int locked;
+ struct mutex lock;
+};
+
+static void block2mtd_setupasync_tryadd(void *data, async_cookie_t cookie)
+{
+ struct block2mtd_setupasync_params *params = data;
+
+ mutex_lock(&params->lock);
+ if (add_device(params->name, params->erase_size) != NULL)
+ params->attached = 1;
+ mutex_unlock(&params->lock);
+}
+
+static int block2mtd_setupasync(void *p)
+{
+ struct block2mtd_setupasync_params *params = p;
+ int i;
+
+ printk(KERN_WARNING "block2mtd: spawned kernel thread for async waiting on '%s'\n", params->name);
+ mutex_init(&params->lock);
+ for (i=0; i<20 && params->attached == 0; i++) {
+ int attached = 0;
+ msleep(1000);
+
+ /* Use mutex to synchronize with async thread */
+ mutex_lock(&params->lock);
+ attached = params->attached;
+ mutex_unlock(&params->lock);
+ /* HACK: Call add_device from async_schedule()ed task
+ so mounting rootfs will wait for ubi scan to complete */
+ if (attached == 0)
+ async_schedule(block2mtd_setupasync_tryadd, p);
+ }
+ kfree(params->name);
+ kfree(params);
+
+ return 0;
+}

-static int block2mtd_setup2(const char *val)
+static int block2mtd_setup2(const char *val, int async)
{
char buf[80 + 12]; /* 80 for device, 12 for erase size */
char *str = buf;
@@ -382,6 +428,7 @@
char *name;
size_t erase_size = PAGE_SIZE;
int i, ret;
+ struct block2mtd_setupasync_params *params;

if (strnlen(val, sizeof(buf)) >= sizeof(buf))
parse_err("parameter too long");
@@ -409,16 +456,32 @@
}
}

- add_device(name, erase_size);
+ if (add_device(name, erase_size) != NULL)
+ return 0;
+
+ params = kzalloc(sizeof(struct block2mtd_setupasync_params), GFP_KERNEL);
+ if (!params)
+ return 0;
+
+ params->name = kmalloc(strlen(name)+1, GFP_KERNEL);
+ params->erase_size = erase_size;
+ if (!params->name) {
+ kfree(params);
+ return 0;
+ }
+
+ memcpy(params->name, name, strlen(name)+1);
+
+ if (async)
+ kthread_run(block2mtd_setupasync, params, "block2mtd/setupasync");

return 0;
}

-
static int block2mtd_setup(const char *val, struct kernel_param *kp)
{
#ifdef MODULE
- return block2mtd_setup2(val);
+ return block2mtd_setup2(val, 0);
#else
/* If more parameters are later passed in via
/sys/module/block2mtd/parameters/block2mtd
@@ -426,7 +489,7 @@
we can parse the argument now. */

if (block2mtd_init_called)
- return block2mtd_setup2(val);
+ return block2mtd_setup2(val, 0);

/* During early boot stage, we only save the parameters
here. We must parse them later: if the param passed
@@ -451,7 +514,7 @@

#ifndef MODULE
if (strlen(block2mtd_paramline))
- ret = block2mtd_setup2(block2mtd_paramline);
+ ret = block2mtd_setup2(block2mtd_paramline, 1);
block2mtd_init_called = 1;
#endif

Index: linux-2.6.31-rc3/drivers/mtd/ubi/build.c
===================================================================
--- linux-2.6.31-rc3.orig/drivers/mtd/ubi/build.c 2009-07-15 14:46:41.094276489 +0200
+++ linux-2.6.31-rc3/drivers/mtd/ubi/build.c 2009-07-15 22:54:48.837303164 +0200
@@ -42,6 +42,7 @@
#include <linux/log2.h>
#include <linux/kthread.h>
#include <linux/reboot.h>
+#include <linux/delay.h>
#include "ubi.h"

/* Maximum length of the 'mtd=' parameter */
@@ -122,6 +123,30 @@
static struct device_attribute dev_mtd_num =
__ATTR(mtd_num, S_IRUGO, dev_attribute_show, NULL);

+static void ubi_notify_remove(struct mtd_info *mtd)
+{
+
+}
+
+static void ubi_notify_add(struct mtd_info *mtd)
+{
+ int err;
+
+ if (mtd->type == MTD_ABSENT)
+ return;
+
+ printk(KERN_WARNING "ubi_notify_add(%p [%s])\n", mtd, mtd->name);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO, 0);
+ if (err < 0) {
+ printk(KERN_WARNING "ubi_attach failed on '%s': %d\n", mtd->name, err);
+ }
+}
+
+static struct mtd_notifier ubi_notifier = {
+ .add = ubi_notify_add,
+ .remove = ubi_notify_remove,
+};
+
/**
* ubi_volume_notify - send a volume change notification.
* @ubi: UBI device description object
@@ -1142,6 +1167,40 @@
return mtd;
}

+static int ubi_init_attach_mtd(void *data)
+{
+ struct mtd_dev_param *p = data;
+ struct mtd_info *mtd;
+ int err;
+ int retries = 200;
+
+ printk(KERN_WARNING "ubi_init_attach_mtd(%s): waiting async for device to appear\n", p->name);
+ do {
+ mtd = open_mtd_device(p->name);
+ if (IS_ERR(mtd)) {
+ if (retries-- > 0) {
+ msleep(100);
+ continue;
+ }
+ err = PTR_ERR(mtd);
+ ubi_err("cannot attach mtd %s: %d\n", p->name, err);
+ return 0;
+ }
+ break;
+ } while (1);
+
+ mutex_lock(&ubi_devices_mutex);
+ err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
+ p->vid_hdr_offs);
+ mutex_unlock(&ubi_devices_mutex);
+ if (err < 0) {
+ put_mtd_device(mtd);
+ ubi_err("cannot attach mtd%d: %d", mtd->index, err);
+ }
+
+ return 0;
+}
+
static int __init ubi_init(void)
{
int err, i, k;
@@ -1181,29 +1240,12 @@
if (!ubi_wl_entry_slab)
goto out_dev_unreg;

+ register_mtd_user(&ubi_notifier);
+#if 0
/* Attach MTD devices */
- for (i = 0; i < mtd_devs; i++) {
- struct mtd_dev_param *p = &mtd_dev_param[i];
- struct mtd_info *mtd;
-
- cond_resched();
-
- mtd = open_mtd_device(p->name);
- if (IS_ERR(mtd)) {
- err = PTR_ERR(mtd);
- goto out_detach;
- }
-
- mutex_lock(&ubi_devices_mutex);
- err = ubi_attach_mtd_dev(mtd, UBI_DEV_NUM_AUTO,
- p->vid_hdr_offs);
- mutex_unlock(&ubi_devices_mutex);
- if (err < 0) {
- put_mtd_device(mtd);
- ubi_err("cannot attach mtd%d", mtd->index);
- goto out_detach;
- }
- }
+ for (i = 0; i < mtd_devs; i++)
+ kthread_run(ubi_init_attach_mtd, &mtd_dev_param[i], "ubi/mtdprobe%d", i);
+#endif

return 0;

@@ -1231,6 +1273,7 @@
{
int i;

+ unregister_mtd_user(&ubi_notifier);
for (i = 0; i < UBI_MAX_DEVICES; i++)
if (ubi_devices[i]) {
mutex_lock(&ubi_devices_mutex);

--
Tobias PGP: http://9ac7e0bc.uguu.de

2009-07-19 06:01:20

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: block2mtd and ubi are initialized too early when compiled in on 2.6.31-rc2

On Thu, 2009-07-16 at 16:28 +0200, Tobias Diedrich wrote:
> Artem Bityutskiy wrote:
> > Hi,
> >
> > On Wed, 2009-07-15 at 13:52 +0200, Tobias Diedrich wrote:
> > > On 2.6.31-rc2 the block2mtd drivers module_init is called before any
> > > block devices have been registered.
> >
> > Hmm, ok. Is this because block devices are registered asynchronously?
> > Could you please point to the code where it is done, just for reference.
>
> AFAICS yes.
> All module_init()s are called asynchronously from init/main.c:do_initcalls()

They are done in a kernel thread, but sequentially, one after the other.
So there is no real synchronicity there.

I think the real problem is that block-devices are probed/initialized
asynchronously. E.g., see 'ata_host_register()'. They call
'async_schedule()' which is doing the slow initialization in a separate
thread.

Thus, you probably should to try to add something like
'async_synchronize_full()' to the blk2mtd driver and wait for block
devices to initialize. Or indeed 'wait_for_device_probe()' may be what
you need.

--
Best regards,
Artem Bityutskiy (Битюцкий Артём)