[PATCH] ubi: init even if mtd device cannot be attached, if built into kernel

[PATCH] ubi: init even if mtd device cannot be attached, if built into kernel

[Marc Kleine-Budde]

Ubi can be built into the kernel or be compiled as a kernel module.
Further on the command line one can specify mtd devices to be attach to
ubi while loading. In the current implementation the ubi driver refuses
to load if one of the mtd devices cannot be attached.

Consider:
1) ubi compiled into the kernel and
2) a mtd device specified on the command line and
3) this mtd device contains bogus data (for whatever reason).

During init ubi tries to attach the mtd device is this fails the whole
ubi subsystem isn't initialized. Later the userspace cannot attach any
mtd to ubi because ubi isn't loaded.

This patch keeps the current behaviour: if ubi is compiled as a module
and a mtd device cannot be attached the ubi module cannot be loaded,
but changes it for the ubi-is-built-into-the-kernel usecase.

If ubi is builtin, a not attachable mtd device doen't stop ubi from
initializing. This slightly modifies the behaviour if multiple mtd
devices are specified on the command line. Now every mtd device is
probed and, if possible, attached, i.e. a faulty mtd device doesn't
stop the others from being attached.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/mtd/ubi/build.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 14cec04..2f6f09f 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -123,6 +123,12 @@ static struct device_attribute dev_bgt_enabled =
 static struct device_attribute dev_mtd_num =
 	__ATTR(mtd_num, S_IRUGO, dev_attribute_show, NULL);
 
+#ifdef CONFIG_MTD_UBI_MODULE
+static inline int ubi_is_module(void) { return 1; }
+#else
+static inline int ubi_is_module(void) { return 0; }
+#endif
+
 /**
  * ubi_volume_notify - send a volume change notification.
  * @ubi: UBI device description object
@@ -1200,9 +1206,11 @@ static int __init ubi_init(void)
 					 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;
+			if (ubi_is_module()) {
+				put_mtd_device(mtd);
+				goto out_detach;
+			}
 		}
 	}
 
-- 
1.7.1

Re: [PATCH] ubi: init even if mtd device cannot be attached, if built into kernel

[Artem Bityutskiy]

On Thu, 2010-04-29 at 12:41 +0200, Marc Kleine-Budde wrote:
> Ubi can be built into the kernel or be compiled as a kernel module.
> Further on the command line one can specify mtd devices to be attach to
> ubi while loading. In the current implementation the ubi driver refuses
> to load if one of the mtd devices cannot be attached.
> 
> Consider:
> 1) ubi compiled into the kernel and
> 2) a mtd device specified on the command line and
> 3) this mtd device contains bogus data (for whatever reason).
> 
> During init ubi tries to attach the mtd device is this fails the whole
> ubi subsystem isn't initialized. Later the userspace cannot attach any
> mtd to ubi because ubi isn't loaded.
> 
> This patch keeps the current behaviour: if ubi is compiled as a module
> and a mtd device cannot be attached the ubi module cannot be loaded,
> but changes it for the ubi-is-built-into-the-kernel usecase.
> 
> If ubi is builtin, a not attachable mtd device doen't stop ubi from
> initializing. This slightly modifies the behaviour if multiple mtd
> devices are specified on the command line. Now every mtd device is
> probed and, if possible, attached, i.e. a faulty mtd device doesn't
> stop the others from being attached.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/mtd/ubi/build.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 14cec04..2f6f09f 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -123,6 +123,12 @@ static struct device_attribute dev_bgt_enabled =
>  static struct device_attribute dev_mtd_num =
>  	__ATTR(mtd_num, S_IRUGO, dev_attribute_show, NULL);
>  
> +#ifdef CONFIG_MTD_UBI_MODULE
> +static inline int ubi_is_module(void) { return 1; }
> +#else
> +static inline int ubi_is_module(void) { return 0; }
> +#endif

I really hate these ifdefs. Dunno why, but they feel disgusting.

I understand your issue and agree that is should be fixed. And I cannot
really see a better solution. So if no-one complains, I'll accept your
patch.

However, for consistency with other UBI code (see debug.h), please, do
this like

#ifdef CONFIG_MTD_UBI_MODULE
#define ubi_is_module() 1
#else
#define ubi_is_module() 1
#endif

Not a big deal, just to be consistent and have uniform code style.

Thanks... :-(

> +
>  /**
>   * ubi_volume_notify - send a volume change notification.
>   * @ubi: UBI device description object
> @@ -1200,9 +1206,11 @@ static int __init ubi_init(void)
>  					 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;
> +			if (ubi_is_module()) {
> +				put_mtd_device(mtd);
> +				goto out_detach;
> +			}
>  		}
>  	}
>  

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

Re: [PATCH] ubi: init even if mtd device cannot be attached, if built into kernel

[Artem Bityutskiy]

On Wed, 2010-05-05 at 08:12 +0300, Artem Bityutskiy wrote:
> > +#ifdef CONFIG_MTD_UBI_MODULE
> > +static inline int ubi_is_module(void) { return 1; }
> > +#else
> > +static inline int ubi_is_module(void) { return 0; }
> > +#endif
> 
> I really hate these ifdefs. Dunno why, but they feel disgusting.
> 
> I understand your issue and agree that is should be fixed. And I cannot
> really see a better solution. So if no-one complains, I'll accept your
> patch.
> 
> However, for consistency with other UBI code (see debug.h), please, do
> this like
> 
> #ifdef CONFIG_MTD_UBI_MODULE
> #define ubi_is_module() 1
> #else
> #define ubi_is_module() 1
> #endif

Err, of course the second one should be 0, not 1. I can actually do this
modification myself, or you can re-send your patch.

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

Re: [PATCH] ubi: init even if mtd device cannot be attached, if built into kernel

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

Artem Bityutskiy wrote:
> On Wed, 2010-05-05 at 08:12 +0300, Artem Bityutskiy wrote:
>>> +#ifdef CONFIG_MTD_UBI_MODULE
>>> +static inline int ubi_is_module(void) { return 1; }
>>> +#else
>>> +static inline int ubi_is_module(void) { return 0; }
>>> +#endif
>> I really hate these ifdefs. Dunno why, but they feel disgusting.
>>
>> I understand your issue and agree that is should be fixed. And I cannot
>> really see a better solution. So if no-one complains, I'll accept your
>> patch.
>>
>> However, for consistency with other UBI code (see debug.h), please, do
>> this like
>>
>> #ifdef CONFIG_MTD_UBI_MODULE
>> #define ubi_is_module() 1
>> #else
>> #define ubi_is_module() 1
>> #endif
> 
> Err, of course the second one should be 0, not 1. I can actually do this
> modification myself, or you can re-send your patch.

I'll resend a patch.

Cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]