Patchwork [v3,0/6] UBI: add max_beb_per1024 parameter / ioctl

login
register
mail settings
Submitter Artem Bityutskiy
Date Aug. 23, 2012, 12:30 p.m.
Message ID <1345725030.2848.236.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/179618/
State New
Headers show

Comments

Artem Bityutskiy - Aug. 23, 2012, 12:30 p.m.
From fa7595564e95672ab87d4a910c6bc02123b75e88 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Thu, 23 Aug 2012 13:01:11 +0300
Subject: [PATCH] UBI: disallow max_beb_per1024 on NOR flash

If the flash does not admit of bad blocks (e.g., NOR) - disallow using
'max_beb_per1024'.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---

Second attempt, did not test :-)

 drivers/mtd/ubi/build.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
Shmulik Ladkani - Aug. 23, 2012, 12:44 p.m.
Hi,

On Thu, 23 Aug 2012 15:30:30 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From fa7595564e95672ab87d4a910c6bc02123b75e88 Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Thu, 23 Aug 2012 13:01:11 +0300
> Subject: [PATCH] UBI: disallow max_beb_per1024 on NOR flash
> 
> If the flash does not admit of bad blocks (e.g., NOR) - disallow using
> 'max_beb_per1024'.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
> 
> Second attempt, did not test :-)

Looks ok to me.

Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Artem Bityutskiy - Aug. 23, 2012, 12:58 p.m.
On Thu, 2012-08-23 at 15:44 +0300, Shmulik Ladkani wrote:
> Hi,
> 
> On Thu, 23 Aug 2012 15:30:30 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > From fa7595564e95672ab87d4a910c6bc02123b75e88 Mon Sep 17 00:00:00 2001
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > Date: Thu, 23 Aug 2012 13:01:11 +0300
> > Subject: [PATCH] UBI: disallow max_beb_per1024 on NOR flash
> > 
> > If the flash does not admit of bad blocks (e.g., NOR) - disallow using
> > 'max_beb_per1024'.
> > 
> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> > ---
> > 
> > Second attempt, did not test :-)
> 
> Looks ok to me.
> 
> Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Thanks, it is pushed now. The further things we'd need to do are:

1. Somewhat test what we ended up with.
2. Write some documentation.
3. Search for UBI users in the mtd mailing list and send them
   an e-mail with heads-up, to make sure people have a possibility
   to complain before this stuff is merged.

I can do number 3 :-)
Richard Genoud - Aug. 23, 2012, 1:13 p.m.
2012/8/23 Artem Bityutskiy <dedekind1@gmail.com>:
> Thanks, it is pushed now. The further things we'd need to do are:
>
> 1. Somewhat test what we ended up with.
> 2. Write some documentation.
> 3. Search for UBI users in the mtd mailing list and send them
>    an e-mail with heads-up, to make sure people have a possibility
>    to complain before this stuff is merged.
>
> I can do number 3 :-)

I could do some more tests with the final version.

I guess that the documentation to update is there:
http://www.linux-mtd.infradead.org/doc/ubi.html
The message commit of dccaefbd68ce0c4d83f1d57667726868bdc03cce (UBI:
use the whole MTD device size to get bad_peb_limit)
could be a start for the "Why?"
Artem Bityutskiy - Aug. 23, 2012, 2:45 p.m.
On Thu, 2012-08-23 at 15:13 +0200, Richard Genoud wrote:
> I could do some more tests with the final version.

Thanks!

> I guess that the documentation to update is there:
> http://www.linux-mtd.infradead.org/doc/ubi.html

Yes, a separate section about how UBI reserves eraseblocks for bad PEBs
handling is needed.

> The message commit of dccaefbd68ce0c4d83f1d57667726868bdc03cce (UBI:
> use the whole MTD device size to get bad_peb_limit)
> could be a start for the "Why?"

Sure.

Additionally, all places where we mention 2% should be amended
correspondingly. I guess this commit can be used to locate the places:
http://git.infradead.org/mtd-www.git/commit/2b91234cf1dd9968d18679b19d7dbe22b6508cff

Also, this section should be amended:
http://www.linux-mtd.infradead.org/doc/ubi.html#L_overhead

Patch

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 355756b..aaf7df3 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -646,8 +646,18 @@  static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	ubi->flash_size = ubi->mtd->size;
 
 	if (mtd_can_have_bb(ubi->mtd)) {
-		ubi->bad_allowed = 1;
+		if (max_beb_per1024 < 0 ||
+		    max_beb_per1024 > MAX_MTD_UBI_BEB_LIMIT)
+			return -EINVAL;
+		if (!max_beb_per1024)
+			max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
+
 		ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
+		ubi->bad_allowed = 1;
+	} else if (max_beb_per1024) {
+		ubi_err("mtd%d does not admit of bad blocks, max_beb_per1024 "
+			"cannot be used", mtd->index);
+		return -EINVAL;
 	}
 
 	if (ubi->mtd->type == MTD_NORFLASH) {
@@ -857,12 +867,6 @@  int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
 	struct ubi_device *ubi;
 	int i, err, ref = 0;
 
-	if (max_beb_per1024 < 0 || max_beb_per1024 > MAX_MTD_UBI_BEB_LIMIT)
-		return -EINVAL;
-
-	if (!max_beb_per1024)
-		max_beb_per1024 = CONFIG_MTD_UBI_BEB_LIMIT;
-
 	/*
 	 * Check if we already have the same MTD device attached.
 	 *