[PATCH 0/5] ubi: Fix bad PEBs reserve caclulation

Discussion:

Shmulik Ladkani

2012-07-04 08:05:59 UTC

Permalink

--
1.7.9

Shmulik Ladkani

2012-07-04 08:06:00 UTC

Permalink

Introduce 'ubi->bad_peb_limit', which specifies an upper limit of PEBs
ubi expects to go bad.
Currently, it is initialized to a fixed percentage of total PEBs in the
ubi device (configurable via CONFIG_MTD_UBI_BEB_LIMIT).

The 'bad_peb_limit' is intended to be used for calculating the amount of
PEBs ubi needs to reserve for bad eraseblock handling.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>
---
arch/arm/configs/sam9_l9260_defconfig | 1 +
drivers/mtd/ubi/Kconfig | 11 +++++++++++
drivers/mtd/ubi/build.c | 13 ++++++++++++-
drivers/mtd/ubi/ubi.h | 2 ++
4 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/arch/arm/configs/sam9_l9260_defconfig b/arch/arm/configs/sam9_l9260_defconfig
index ecf2531..f6917c9 100644
--- a/arch/arm/configs/sam9_l9260_defconfig
+++ b/arch/arm/configs/sam9_l9260_defconfig
@@ -40,6 +40,7 @@ CONFIG_MTD_NAND_ATMEL=y
CONFIG_MTD_NAND_PLATFORM=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_BEB_RESERVE=3
+CONFIG_MTD_UBI_BEB_LIMIT=3
CONFIG_MTD_UBI_GLUEBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 738ee8d..8df256f 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -42,6 +42,17 @@ config MTD_UBI_BEB_RESERVE
eraseblocks (e.g. NOR flash), this value is ignored and nothing is
reserved. Leave the default value if unsure.

+config MTD_UBI_BEB_LIMIT
+ int "Percentage of maximum expected bad eraseblocks"
+ default 2
+ range 0 25
+ help
+ This option specifies the maximum bad eraseblocks UBI expects on the
+ ubi device (percents of total number of flash eraseblocks).
+ If the underlying flash does not admit of bad eraseblocks (e.g. NOR
+ flash), this value is ignored.
+ Leave the default value if unsure.
+
config MTD_UBI_GLUEBI
tristate "MTD devices emulation driver (gluebi)"
help
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 2c5ed5c..62cc6ce 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -607,8 +607,19 @@ static int io_init(struct ubi_device *ubi)
ubi->peb_count = mtd_div_by_eb(ubi->mtd->size, ubi->mtd);
ubi->flash_size = ubi->mtd->size;

- if (mtd_can_have_bb(ubi->mtd))
+ if (mtd_can_have_bb(ubi->mtd)) {
ubi->bad_allowed = 1;
+ if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
+ int percent = CONFIG_MTD_UBI_BEB_LIMIT;
+ int beb_limit;
+
+ beb_limit = mult_frac(ubi->peb_count, percent, 100);
+ /* round it up */
+ if (mult_frac(beb_limit, 100, percent) < ubi->peb_count)
+ beb_limit++;
+ ubi->bad_peb_limit = beb_limit;
+ }
+ }

if (ubi->mtd->type == MTD_NORFLASH) {
ubi_assert(ubi->mtd->writesize == 1);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index a1a81c9..b5217ef 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -363,6 +363,7 @@ struct ubi_wl_entry;
* @flash_size: underlying MTD device size (in bytes)
* @peb_count: count of physical eraseblocks on the MTD device
* @peb_size: physical eraseblock size
+ * @bad_peb_limit: top limit of expected bad physical eraseblocks
* @bad_peb_count: count of bad physical eraseblocks
* @good_peb_count: count of good physical eraseblocks
* @corr_peb_count: count of corrupted physical eraseblocks (preserved and not
@@ -410,6 +411,7 @@ struct ubi_device {
int avail_pebs;
int beb_rsvd_pebs;
int beb_rsvd_level;
+ int bad_peb_limit;

int autoresize_vol_id;
int vtbl_slots;

--
1.7.9

Artem Bityutskiy

2012-07-18 07:09:19 UTC

Permalink

Post by Shmulik Ladkani
Introduce 'ubi->bad_peb_limit', which specifies an upper limit of PEBs
ubi expects to go bad.
Currently, it is initialized to a fixed percentage of total PEBs in the
ubi device (configurable via CONFIG_MTD_UBI_BEB_LIMIT).
The 'bad_peb_limit' is intended to be used for calculating the amount of
PEBs ubi needs to reserve for bad eraseblock handling.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>

Created branch "beb" and pushed this patch there with minor amendments
(see diff below). Let's use this branch in UBI tree for this work and
merge it as soon as it is ready.

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index c33e25b..dee90b7 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -668,13 +668,12 @@ static int io_init(struct ubi_device *ubi)
ubi->bad_allowed = 1;
if (CONFIG_MTD_UBI_BEB_LIMIT > 0) {
int percent = CONFIG_MTD_UBI_BEB_LIMIT;
- int beb_limit;
+ int limit = mult_frac(ubi->peb_count, percent, 100);

- beb_limit = mult_frac(ubi->peb_count, percent, 100);
- /* round it up */
- if (mult_frac(beb_limit, 100, percent) < ubi->peb_count)
- beb_limit++;
- ubi->bad_peb_limit = beb_limit;
+ /* Round it up */
+ if (mult_frac(limit, 100, percent) < ubi->peb_count)
+ limit += 1;
+ ubi->bad_peb_limit = limit;
}
}
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/fff33c14/attachment-0001.sig>

Artem Bityutskiy

2012-07-18 10:40:53 UTC

Permalink

I've also amended the Kconfig text a tiny bit and dropped the defconfig
changes - let's have them separately as a single patch at the end of the
series.
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/544983b4/attachment.sig>

Shmulik Ladkani

2012-07-19 06:16:00 UTC

Permalink

Post by Artem Bityutskiy
I've also amended the Kconfig text a tiny bit and dropped the defconfig
changes - let's have them separately as a single patch at the end of the
series.

Wouldn't having the defconfig change as the last patch break things for
those defconfigs that had explicitly set CONFIG_MTD_UBI_BEB_RESERVE
other than the default?

Meaning, if the one-before-last would be "kill CONFIG_MTD_UBI_BEB_RESERVE",
then those defconfigs that had _explicitly_ set a BEB_RESERVE value,
which do not YET set a BEB_LIMIT value, will have their BEB_LIMIT as
the default - but they actually meant a specific value other than the
default.

This is why I tried to:
- set the CONFIG_MTD_UBI_BEB_LIMIT in defconfigs as part of the commit
which introduces this config (copy same value as their RESERVE config)
- kill all CONFIG_MTD_UBI_BEB_RESERVE references from defconfigs as part
of the commit which kills it

Regards,
Shmulik

Artem Bityutskiy

2012-07-30 13:00:56 UTC

Permalink

Post by Shmulik Ladkani

Post by Artem Bityutskiy
I've also amended the Kconfig text a tiny bit and dropped the defconfig
changes - let's have them separately as a single patch at the end of the
series.

Wouldn't having the defconfig change as the last patch break things for
those defconfigs that had explicitly set CONFIG_MTD_UBI_BEB_RESERVE
other than the default?

OK, fair enough, but let's have it as a 2 separate patches anyway. It is
not a big deal to first change the defconfig, and then actually add the
new option.
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120730/a9d45a61/attachment-0001.sig>

Shmulik Ladkani

2012-07-04 08:06:02 UTC

Permalink

CONFIG_MTD_UBI_BEB_RESERVE and MIN_RESEVED_PEBS are no longer used,
since the amount of reserved eraseblocks for bad PEB handling is now
derived from 'ubi->bad_peb_limit' (ubi's maximum expected bad
eraseblocks).

Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>
---
arch/arm/configs/sam9_l9260_defconfig | 1 -
drivers/mtd/ubi/Kconfig | 15 ---------------
drivers/mtd/ubi/ubi.h | 3 ---
3 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/arch/arm/configs/sam9_l9260_defconfig b/arch/arm/configs/sam9_l9260_defconfig
index f6917c9..da276f9 100644
--- a/arch/arm/configs/sam9_l9260_defconfig
+++ b/arch/arm/configs/sam9_l9260_defconfig
@@ -39,7 +39,6 @@ CONFIG_MTD_NAND=y
CONFIG_MTD_NAND_ATMEL=y
CONFIG_MTD_NAND_PLATFORM=y
CONFIG_MTD_UBI=y
-CONFIG_MTD_UBI_BEB_RESERVE=3
CONFIG_MTD_UBI_BEB_LIMIT=3
CONFIG_MTD_UBI_GLUEBI=y
CONFIG_BLK_DEV_LOOP=y
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 7eb91cb..145cda2 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -27,21 +27,6 @@ config MTD_UBI_WL_THRESHOLD
life-cycle less than 10000, the threshold should be lessened (e.g.,
to 128 or 256, although it does not have to be power of 2).

-config MTD_UBI_BEB_RESERVE
- int "Percentage of reserved eraseblocks for bad eraseblocks handling"
- default 1
- range 0 25
- help
- If the MTD device admits of bad eraseblocks (e.g. NAND flash), UBI
- reserves some amount of physical eraseblocks to handle new bad
- eraseblocks. For example, if a flash physical eraseblock becomes bad,
- UBI uses these reserved physical eraseblocks to relocate the bad one.
- This option specifies how many physical eraseblocks will be reserved
- for bad eraseblock handling (percents of total number of good flash
- eraseblocks). If the underlying flash does not admit of bad
- eraseblocks (e.g. NOR flash), this value is ignored and nothing is
- reserved. Leave the default value if unsure.
-
config MTD_UBI_BEB_LIMIT
int "Percentage of maximum expected bad eraseblocks"
default 2
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index b5217ef..80c394eb 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -59,9 +59,6 @@
#define ubi_err(fmt, ...) printk(KERN_ERR "UBI error: %s: " fmt "\n", \
__func__, ##__VA_ARGS__)

-/* Lowest number PEBs reserved for bad PEB handling */
-#define MIN_RESEVED_PEBS 2
-
/* Background thread name pattern */
#define UBI_BGT_NAME_PATTERN "ubi_bgt%dd"

--
1.7.9

Shmulik Ladkani

2012-07-04 08:06:03 UTC

Permalink

The function name within the comment was not aligned with the actual
function name.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>
---
drivers/mtd/ubi/misc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ubi/misc.c b/drivers/mtd/ubi/misc.c
index e9dcb83..25b564f 100644
--- a/drivers/mtd/ubi/misc.c
+++ b/drivers/mtd/ubi/misc.c
@@ -92,7 +92,7 @@ int ubi_check_volume(struct ubi_device *ubi, int vol_id)
}

/**
- * ubi_calculate_rsvd_pool - calculate how many PEBs must be reserved for bad
+ * ubi_calculate_reserved - calculate how many PEBs must be reserved for bad
* eraseblock handling.
* @ubi: UBI device description object
*/

--
1.7.9

Artem Bityutskiy

2012-07-18 10:38:08 UTC

Permalink

Post by Shmulik Ladkani
The function name within the comment was not aligned with the actual
function name.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>

Pushed this one directly to the master branch, thanks!
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/72766aaa/attachment.sig>

Shmulik Ladkani

2012-07-04 08:06:01 UTC

Permalink

The existing mechanism of reserving PEBs for bad PEB handling has two
flaws:
- It is calculated as a percentage of good PEBs instead of total PEBs.
- There's no limit on the amount of PEBs UBI reserves for future bad
eraseblock handling.

This patch changes the mechanism to overcome these flaws.

The desired level of PEBs reserved for bad PEB handling (beb_rsvd_level)
is set to the maximum expected bad eraseblocks (bad_peb_limit) minus the
existing number of bad eraseblocks (bad_peb_count).

The actual amount of PEBs reserved for bad PEB handling is usually set
to the desired level (but in some circumstances may be lower than the
desired level, e.g. when attaching to a device that has too few
available PEBs to satisfy the desired level).

In the case where the device has too many bad PEBs (above the expected
limit), then the desired level, and the actual amount of PEBs reserved
are set to zero. No PEBs will be set aside for future bad eraseblock
handling - even if some PEBs are made available (e.g. by shrinking a
volume).
If another PEB goes bad, and there are available PEBs, then the
eraseblock will be marked bad (consuming one available PEB). But if
there are no available PEBs, ubi will go into readonly mode.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>
---
drivers/mtd/ubi/Kconfig | 8 ++++++++
drivers/mtd/ubi/misc.c | 16 ++++++++++++----
drivers/mtd/ubi/wl.c | 44 ++++++++++++++++++++++++++++----------------
3 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 8df256f..7eb91cb 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -47,8 +47,16 @@ config MTD_UBI_BEB_LIMIT
default 2
range 0 25
help
+ If the MTD device admits of bad eraseblocks (e.g. NAND flash), UBI
+ reserves some amount of physical eraseblocks to handle new bad
+ eraseblocks.
This option specifies the maximum bad eraseblocks UBI expects on the
ubi device (percents of total number of flash eraseblocks).
+ This limit is used in order to derive amount of eraseblock UBI
+ reserves for handling new bad blocks.
+ If the device has more bad eraseblocks than this limit, UBI does not
+ reserve any physical eraseblocks for new bad eraseblocks, but
+ attempts to use available eraseblocks (if any).
If the underlying flash does not admit of bad eraseblocks (e.g. NOR
flash), this value is ignored.
Leave the default value if unsure.
diff --git a/drivers/mtd/ubi/misc.c b/drivers/mtd/ubi/misc.c
index f6a7d7a..e9dcb83 100644
--- a/drivers/mtd/ubi/misc.c
+++ b/drivers/mtd/ubi/misc.c
@@ -98,10 +98,18 @@ int ubi_check_volume(struct ubi_device *ubi, int vol_id)
*/
void ubi_calculate_reserved(struct ubi_device *ubi)
{
- ubi->beb_rsvd_level = ubi->good_peb_count/100;
- ubi->beb_rsvd_level *= CONFIG_MTD_UBI_BEB_RESERVE;
- if (ubi->beb_rsvd_level < MIN_RESEVED_PEBS)
- ubi->beb_rsvd_level = MIN_RESEVED_PEBS;
+ /*
+ * Calculate the actual number of PEBs currently needed to be reserved
+ * for future bad eraseblock handling.
+ */
+ ubi->beb_rsvd_level = ubi->bad_peb_limit - ubi->bad_peb_count;
+ if (ubi->beb_rsvd_level < 0) {
+ ubi->beb_rsvd_level = 0;
+ ubi_warn("number of bad PEBs (%d) is above the expected limit "
+ "(%d), not reserving any PEBs for bad PEB handling, "
+ "will use available PEBs (if any)",
+ ubi->bad_peb_count, ubi->bad_peb_limit);
+ }
}

/**
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index b6be644..9143f35 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -978,9 +978,10 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
int cancel)
{
struct ubi_wl_entry *e = wl_wrk->e;
- int pnum = e->pnum, err, need;
+ int pnum = e->pnum, err;
int vol_id = wl_wrk->vol_id;
int lnum = wl_wrk->lnum;
+ int available_consumed = 0;

if (cancel) {
dbg_wl("cancel erasure of PEB %d EC %d", pnum, e->ec);
@@ -1045,20 +1046,14 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
}

spin_lock(&ubi->volumes_lock);
- need = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs + 1;
- if (need > 0) {
- need = ubi->avail_pebs >= need ? need : ubi->avail_pebs;
- ubi->avail_pebs -= need;
- ubi->rsvd_pebs += need;
- ubi->beb_rsvd_pebs += need;
- if (need > 0)
- ubi_msg("reserve more %d PEBs", need);
- }
-
if (ubi->beb_rsvd_pebs == 0) {
- spin_unlock(&ubi->volumes_lock);
- ubi_err("no reserved physical eraseblocks");
- goto out_ro;
+ if (ubi->avail_pebs == 0) {
+ spin_unlock(&ubi->volumes_lock);
+ ubi_err("no reserved/available physical eraseblocks");
+ goto out_ro;
+ }
+ ubi->avail_pebs -= 1;
+ available_consumed = 1;
}
spin_unlock(&ubi->volumes_lock);

@@ -1068,11 +1063,23 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
goto out_ro;

spin_lock(&ubi->volumes_lock);
- ubi->beb_rsvd_pebs -= 1;
+ if (ubi->beb_rsvd_pebs > 0) {
+ if (available_consumed) {
+ /*
+ * Some PEBs were added to the reserved pool since we
+ * last checked. Use a PEB from the reserved pool.
+ */
+ ubi->avail_pebs += 1;
+ available_consumed = 0;
+ }
+ ubi->beb_rsvd_pebs -= 1;
+ }
ubi->bad_peb_count += 1;
ubi->good_peb_count -= 1;
ubi_calculate_reserved(ubi);
- if (ubi->beb_rsvd_pebs)
+ if (available_consumed)
+ ubi_warn("no PEBs in the reserved pool, used an available PEB");
+ else if (ubi->beb_rsvd_pebs)
ubi_msg("%d PEBs left in the reserve", ubi->beb_rsvd_pebs);
else
ubi_warn("last PEB from the reserved pool was used");
@@ -1081,6 +1088,11 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
return err;

out_ro:
+ if (available_consumed) {
+ spin_lock(&ubi->volumes_lock);
+ ubi->avail_pebs += 1;
+ spin_unlock(&ubi->volumes_lock);
+ }
ubi_ro_mode(ubi);
return err;
}

--
1.7.9

Richard Genoud

2012-07-09 10:15:17 UTC

Permalink

Post by Shmulik Ladkani
diff --git a/drivers/mtd/ubi/misc.c b/drivers/mtd/ubi/misc.c
index f6a7d7a..e9dcb83 100644
--- a/drivers/mtd/ubi/misc.c
+++ b/drivers/mtd/ubi/misc.c
@@ -98,10 +98,18 @@ int ubi_check_volume(struct ubi_device *ubi, int vol_id)
*/
void ubi_calculate_reserved(struct ubi_device *ubi)
{
- ubi->beb_rsvd_level = ubi->good_peb_count/100;
- ubi->beb_rsvd_level *= CONFIG_MTD_UBI_BEB_RESERVE;
- if (ubi->beb_rsvd_level < MIN_RESEVED_PEBS)
- ubi->beb_rsvd_level = MIN_RESEVED_PEBS;
+ /*
+ * Calculate the actual number of PEBs currently needed to be reserved
+ * for future bad eraseblock handling.
+ */
+ ubi->beb_rsvd_level = ubi->bad_peb_limit - ubi->bad_peb_count;
+ if (ubi->beb_rsvd_level < 0) {
+ ubi->beb_rsvd_level = 0;
+ ubi_warn("number of bad PEBs (%d) is above the expected limit "
+ "(%d), not reserving any PEBs for bad PEB handling, "
+ "will use available PEBs (if any)",
+ ubi->bad_peb_count, ubi->bad_peb_limit);
+ }
}

is it ok for beb_rsvd_level to be in the range [0..x[ instead of [2..x[ ?

Shmulik Ladkani

2012-07-09 11:02:32 UTC

Permalink

Post by Richard Genoud

Post by Shmulik Ladkani
+ /*
+ * Calculate the actual number of PEBs currently needed to be reserved
+ * for future bad eraseblock handling.
+ */
+ ubi->beb_rsvd_level = ubi->bad_peb_limit - ubi->bad_peb_count;
+ if (ubi->beb_rsvd_level < 0) {
+ ubi->beb_rsvd_level = 0;
+ ubi_warn("number of bad PEBs (%d) is above the expected limit "
+ "(%d), not reserving any PEBs for bad PEB handling, "
+ "will use available PEBs (if any)",
+ ubi->bad_peb_count, ubi->bad_peb_limit);
+ }
}

is it ok for beb_rsvd_level to be in the range [0..x[ instead of [2..x[ ?

Yes, it is ok in my new scheme.
It is even mandatory, otherwise more and more PEBs will attempt to be
reserved for future bad PEB handling (as 'beb_rsvd_pebs' always wishes
to reach 'beb_rsvd_level') even if passed the limit - this is exactly
what I'd like to fix.

Regards
Shmulik

Artem Bityutskiy

2012-07-18 10:28:37 UTC

Permalink

Post by Shmulik Ladkani
@@ -1045,20 +1046,14 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
}
spin_lock(&ubi->volumes_lock);
- need = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs + 1;
- if (need > 0) {
- need = ubi->avail_pebs >= need ? need : ubi->avail_pebs;
- ubi->avail_pebs -= need;
- ubi->rsvd_pebs += need;
- ubi->beb_rsvd_pebs += need;
- if (need > 0)
- ubi_msg("reserve more %d PEBs", need);
- }
-
if (ubi->beb_rsvd_pebs == 0) {
- spin_unlock(&ubi->volumes_lock);
- ubi_err("no reserved physical eraseblocks");
- goto out_ro;
+ if (ubi->avail_pebs == 0) {
+ spin_unlock(&ubi->volumes_lock);
+ ubi_err("no reserved/available physical eraseblocks");
+ goto out_ro;
+ }
+ ubi->avail_pebs -= 1;
+ available_consumed = 1;
}
spin_unlock(&ubi->volumes_lock);

The whole thing will become simpler if we first mark the PEB as bad
unconditionally (because it _is_ bad), then grab the lock and do all the
re-calculations.

Post by Shmulik Ladkani
@@ -1068,11 +1063,23 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk,
goto out_ro;
spin_lock(&ubi->volumes_lock);
- ubi->beb_rsvd_pebs -= 1;
+ if (ubi->beb_rsvd_pebs > 0) {
+ if (available_consumed) {
+ /*
+ * Some PEBs were added to the reserved pool since we
+ * last checked. Use a PEB from the reserved pool.
+ */
+ ubi->avail_pebs += 1;
+ available_consumed = 0;
+ }
+ ubi->beb_rsvd_pebs -= 1;
+ }
ubi->bad_peb_count += 1;
ubi->good_peb_count -= 1;
ubi_calculate_reserved(ubi);

We do not need to call this function from here, right?
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/71d52508/attachment.sig>

Artem Bityutskiy

2012-07-18 11:01:12 UTC

Permalink

Post by Artem Bityutskiy

Post by Shmulik Ladkani
ubi->bad_peb_count += 1;
ubi->good_peb_count -= 1;
ubi_calculate_reserved(ubi);

We do not need to call this function from here, right?

Err, sorry, it _is_ needed!
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/48d85bc3/attachment-0001.sig>

Shmulik Ladkani

2012-07-18 19:55:37 UTC

Permalink

Hi Artem,

Post by Artem Bityutskiy
The whole thing will become simpler if we first mark the PEB as bad
unconditionally (because it _is_ bad), then grab the lock and do all the
re-calculations.

On first glance that would sound the right thing to do.

Actually, when looked at the original code, which does NOT mark the
block bad when 'beb_rsvd_pebs == 0' (instead, does a 'goto out_ro'),
I initially thought "this is SO wrong, the block IS bad, we MUST mark it
bad, what IS the deal here?".
But then I spent more and more time trying to backtrack the reason for
it - and I think I found a reason, quite an improtant one.

Suppose 'beb_rsvd_pebs == 0'.

In the original scheme, it means that there are NO available PEBs at
all. All good PEBs are "assigned" (to user volumes, layout volume, WL
and EBA).

Now, if one of these PEBs goes bad, and you DO mark it bad, and
decrement the good_peb_count, you'll have a shortage of good PEBs - it
will not match the amount of PEBs assigned to the consumers (user
volumes, layout, WL, EBA).
Meaning, next attach would simply fail with a "no enough physical
eraseblocks" message (grep for these in ubi_wl_init and ubi_eba_init).
You won't even be able to attach anymore. Not good.

However, if you DO NOT mark it bad, but instead go into RO mode, you
should be able to later re-attach because the good_peb_count would fit
(no shortage of PEBs).

So going into RO seems a "less worse" solution than marking bad, for
that particular case.

Anyways, I've really invested some thought into this patch, trying to
cover all sorts of esoteric cases.
I think the logic itself is pretty robust, although I would really
appreciate some reassurances on that...

I agree the code does not "read simple" enough, I tried to make the best
simplifying and adding some comments. Let me know if you'd like some
more polish on it.

I saw you submitted a simplified version, I can surely take a look, but
I'm afraid it lacks the proper treatment discussed above (NOT marking
the bad block as bad when beb_rsvd_pebs is zero).

Let me know.

Many thanks,
Shmulik

Artem Bityutskiy

2012-07-19 03:35:49 UTC

Permalink

Post by Shmulik Ladkani
However, if you DO NOT mark it bad, but instead go into RO mode, you
should be able to later re-attach because the good_peb_count would fit
(no shortage of PEBs).

Yeah, you are right, I'll return to the original patch.
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120719/dbde338c/attachment.sig>

Artem Bityutskiy

2012-07-18 11:40:20 UTC

Permalink

Post by Shmulik Ladkani
The existing mechanism of reserving PEBs for bad PEB handling has two
- It is calculated as a percentage of good PEBs instead of total PEBs.
- There's no limit on the amount of PEBs UBI reserves for future bad
eraseblock handling.

OK, I've modified this patch a lot, did not test, and sent what I have
in the beb branch to you - please, take a look.
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/1d2fa6e0/attachment-0001.sig>

Shmulik Ladkani

2012-07-04 08:06:04 UTC

Permalink

Currently, there are several locations where an attempt to reserve more
PEBs for bad PEB handling is made, with the same code being duplicated.

Harmonize it by introducing 'ubi_update_reserved()'.

Also, improve the debug message issued, making it more descriptive.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani at gmail.com>
---
drivers/mtd/ubi/misc.c | 26 ++++++++++++++++++++++++++
drivers/mtd/ubi/ubi.h | 1 +
drivers/mtd/ubi/vmt.c | 20 ++------------------
3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/ubi/misc.c b/drivers/mtd/ubi/misc.c
index 25b564f..b7d26ed 100644
--- a/drivers/mtd/ubi/misc.c
+++ b/drivers/mtd/ubi/misc.c
@@ -92,6 +92,32 @@ int ubi_check_volume(struct ubi_device *ubi, int vol_id)
}

/**
+ * ubi_update_reserved - update the number of PEBs reserved for bad
+ * eraseblock handling.
+ * @ubi: UBI device description object
+ *
+ * This function calculates the gap between current number of
+ * PEBs reserved for bad eraseblock handling and the required level of PEBs
+ * that must be reserved, and if necessary, reserves more PEBs to fill that
+ * gap, according to availability.
+ *
+ * Should be called with ubi->volumes_lock held.
+ */
+void ubi_update_reserved(struct ubi_device *ubi)
+{
+ int need = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs;
+
+ if (need <= 0 || ubi->avail_pebs == 0)
+ return;
+
+ need = ubi->avail_pebs >= need ? need : ubi->avail_pebs;
+ ubi->avail_pebs -= need;
+ ubi->rsvd_pebs += need;
+ ubi->beb_rsvd_pebs += need;
+ ubi_msg("reserved more %d PEBs for bad PEB handling", need);
+}
+
+/**
* ubi_calculate_reserved - calculate how many PEBs must be reserved for bad
* eraseblock handling.
* @ubi: UBI device description object
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 80c394eb..c94612e 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -646,6 +646,7 @@ int ubi_more_leb_change_data(struct ubi_device *ubi, struct ubi_volume *vol,
int ubi_calc_data_len(const struct ubi_device *ubi, const void *buf,
int length);
int ubi_check_volume(struct ubi_device *ubi, int vol_id);
+void ubi_update_reserved(struct ubi_device *ubi);
void ubi_calculate_reserved(struct ubi_device *ubi);
int ubi_check_pattern(const void *buf, uint8_t patt, int size);

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 0669cff..9169e58 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -443,15 +443,7 @@ int ubi_remove_volume(struct ubi_volume_desc *desc, int no_vtbl)
spin_lock(&ubi->volumes_lock);
ubi->rsvd_pebs -= reserved_pebs;
ubi->avail_pebs += reserved_pebs;
- i = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs;
- if (i > 0) {
- i = ubi->avail_pebs >= i ? i : ubi->avail_pebs;
- ubi->avail_pebs -= i;
- ubi->rsvd_pebs += i;
- ubi->beb_rsvd_pebs += i;
- if (i > 0)
- ubi_msg("reserve more %d PEBs", i);
- }
+ ubi_update_reserved(ubi);
ubi->vol_count -= 1;
spin_unlock(&ubi->volumes_lock);

@@ -558,15 +550,7 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
spin_lock(&ubi->volumes_lock);
ubi->rsvd_pebs += pebs;
ubi->avail_pebs -= pebs;
- pebs = ubi->beb_rsvd_level - ubi->beb_rsvd_pebs;
- if (pebs > 0) {
- pebs = ubi->avail_pebs >= pebs ? pebs : ubi->avail_pebs;
- ubi->avail_pebs -= pebs;
- ubi->rsvd_pebs += pebs;
- ubi->beb_rsvd_pebs += pebs;
- if (pebs > 0)
- ubi_msg("reserve more %d PEBs", pebs);
- }
+ ubi_update_reserved(ubi);
for (i = 0; i < reserved_pebs; i++)
new_mapping[i] = vol->eba_tbl[i];
kfree(vol->eba_tbl);

--
1.7.9

Artem Bityutskiy

2012-07-18 11:32:22 UTC

Permalink

+ need = ubi->avail_pebs >= need ? need : ubi->avail_pebs;

Changed this line to
need = min_t(int, need, ubi->avail_pebs);

and pushed to the master branch. Thanks!
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/3736681c/attachment.sig>

Richard Weinberger

2012-07-04 08:35:42 UTC

Permalink

Post by Shmulik Ladkani
ubi: introduce ubi->bad_peb_limit
ubi: Limit amount of reserved eraseblocks for bad PEB handling
ubi: kill CONFIG_MTD_UBI_BEB_RESERVE
ubi: trivial: fix comment of ubi_calculate_reserved()
ubi: harmonize the update of ubi->beb_rsvd_pebs

Has this patch set an impact on UBI fastmap?
...wondering why I'm CC'd. :-)

Thanks,
//richard

Shmulik Ladkani

2012-07-04 11:33:39 UTC

Permalink

Post by Richard Weinberger
Has this patch set an impact on UBI fastmap?

Not sure yet. Maybe. Need to medidate on this :-)

Post by Richard Weinberger
...wondering why I'm CC'd. :-)

Heads-up.
As this is a bit delicate, you may review if the parts modified relate
to your fastmap work.

Regards,
Shmulik

Richard Genoud

2012-07-06 15:27:59 UTC

Permalink

I've got an oops...
this is my dev-kernel in 3.5-rc5 + some work to be able to boot on my board
NB: If I use ubi_format it's ok.
the mtd1 device has 1984 PEB
the 4 last are UBI reserved + BBT

I didn't test without your patch, but anyway something is wrong there.

# flash_erase /dev/mtd1 0 1980
Erasing 128 Kibyte @ f760000 -- 100 % complete
# ubiattach /dev/ubi_ctrl -m1
[ 35.671875] UBI: attaching mtd1 to ubi0
[ 35.671875] UBI DBG (pid 419): ubi_attach_mtd_dev: sizeof(struct
ubi_ainf_peb) 48
[ 35.679687] UBI DBG (pid 419): ubi_attach_mtd_dev: sizeof(struct
ubi_wl_entry) 20
[ 35.687500] UBI DBG (pid 419): io_init: min_io_size 2048
[ 35.695312] UBI DBG (pid 419): io_init: max_write_size 2048
[ 35.703125] UBI DBG (pid 419): io_init: hdrs_min_io_size 2048
[ 35.703125] UBI DBG (pid 419): io_init: ec_hdr_alsize 2048
[ 35.710937] UBI DBG (pid 419): io_init: vid_hdr_alsize 2048
[ 35.718750] UBI DBG (pid 419): io_init: vid_hdr_offset 2048
[ 35.718750] UBI DBG (pid 419): io_init: vid_hdr_aloffset 2048
[ 35.726562] UBI DBG (pid 419): io_init: vid_hdr_shift 0
[ 35.734375] UBI DBG (pid 419): io_init: leb_start 4096
[ 35.742187] UBI DBG (pid 419): io_init: max_erroneous 198
[ 35.742187] UBI: physical eraseblock size: 131072 bytes (128 KiB)
[ 35.750000] UBI: logical eraseblock size: 126976 bytes
[ 35.757812] UBI: smallest flash I/O unit: 2048
[ 35.757812] UBI: VID header offset: 2048 (aligned 2048)
[ 35.765625] UBI: data offset: 4096
[ 36.210937] UBI DBG (pid 419): scan_all: scanning is finished
[ 36.218750] UBI: empty MTD device detected
[ 36.226562] UBI: max. sequence number: 0
[ 36.226562] UBI: create volume table (copy #1)
[ 36.242187] UBI: create volume table (copy #2)
[ 36.257812] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[ 36.265625] pgd = c7520000
[ 36.265625] [00000000] *pgd=27bb7831, *pte=00000000, *ppte=00000000
[ 36.273437] Internal error: Oops: 17 [#1] ARM
[ 36.273437] CPU: 0 Not tainted (3.5.0-rc5+ #14)
[ 36.273437] PC is at ubi_wl_init+0x138/0x36c
[ 36.273437] LR is at schedule_erase+0x50/0x64
[ 36.273437] pc : [<c0195a7c>] lr : [<c0194388>] psr: 60000013
[ 36.273437] sp : c7501ee8 ip : 00008000 fp : 00000000
[ 36.273437] r10: c753fef8 r9 : c0361364 r8 : ffffffe0
[ 36.273437] r7 : c7537020 r6 : c7537034 r5 : c7538280 r4 : c7bd3540
[ 36.273437] r3 : 00000000 r2 : c7bd3938 r1 : c75382c0 r0 : 00000000
[ 36.273437] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 36.273437] Control: 0005317f Table: 27520000 DAC: 00000015
[ 36.273437] Process ubiattach (pid: 419, stack limit = 0xc7500270)
[ 36.273437] Stack: (0xc7501ee8 to 0xc7502000)
[ 36.273437] 1ee0: 00000000 c7537020 c7bd3540
c7537020 c7bd3540 00000000
[ 36.273437] 1f00: c757caa0 c0009388 c7500000 00000000 00000000
c019800c c7bd3540 00000000
[ 36.273437] 1f20: 00000000 c018d79c c757caa0 00000000 becdaaf8
c757caa0 40186f40 00000003
[ 36.273437] 1f40: c0009388 c018dccc ffffffff 00000001 00000000
00000000 00000000 00000000
[ 36.273437] 1f60: becdaaf8 00000003 40186f40 c0076890 c7ba4dc0
00000003 40186f40 c7ba4dc0
[ 36.273437] 1f80: becdaaf8 c0076900 00000003 00000000 becdaaf8
becdaaf8 40186f40 00000003
[ 36.273437] 1fa0: 00000036 c00091e0 becdaaf8 40186f40 00000003
40186f40 becdaaf8 00000000
[ 36.273437] 1fc0: becdaaf8 40186f40 00000003 00000036 00000002
becdadb4 00000001 00000000
[ 36.273437] 1fe0: b6f39ec4 becdaab0 000098c8 b6f39f08 60000010
00000003 00000000 00000000
[ 36.273437] [<c0195a7c>] (ubi_wl_init+0x138/0x36c) from
[<c019800c>] (ubi_attach+0x74/0xbc)
[ 36.273437] [<c019800c>] (ubi_attach+0x74/0xbc) from [<c018d79c>]
(ubi_attach_mtd_dev+0x1dc/0x4bc)
[ 36.273437] [<c018d79c>] (ubi_attach_mtd_dev+0x1dc/0x4bc) from
[<c018dccc>] (ctrl_cdev_ioctl+0xd4/0x164)
[ 36.273437] [<c018dccc>] (ctrl_cdev_ioctl+0xd4/0x164) from
[<c0076890>] (do_vfs_ioctl+0x270/0x2ac)
[ 36.273437] [<c0076890>] (do_vfs_ioctl+0x270/0x2ac) from
[<c0076900>] (sys_ioctl+0x34/0x54)
[ 36.273437] [<c0076900>] (sys_ioctl+0x34/0x54) from [<c00091e0>]
(ret_fast_syscall+0x0/0x2c)
[ 36.273437] Code: e1a01005 e5930000 ebfb44ff ea000074 (e5983020)
[ 36.281250] ---[ end trace f540180ccfcbf7f6 ]---
Segmentation fault

Post by Shmulik Ladkani

Post by Richard Weinberger
Has this patch set an impact on UBI fastmap?

Not sure yet. Maybe. Need to medidate on this :-)

Post by Richard Weinberger
...wondering why I'm CC'd. :-)

Heads-up.
As this is a bit delicate, you may review if the parts modified relate
to your fastmap work.
Regards,
Shmulik

--
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?

Shmulik Ladkani

2012-07-07 06:14:47 UTC

Permalink

Hi Richard,

Post by Richard Genoud
I've got an oops...
this is my dev-kernel in 3.5-rc5 + some work to be able to boot on my board
NB: If I use ubi_format it's ok.
the mtd1 device has 1984 PEB
the 4 last are UBI reserved + BBT
I didn't test without your patch, but anyway something is wrong there.

Many thanks for testing.

Could you please verify the crash only occurs with the patch?

Can you provide the vmlinux matching this oops, so I may analyze the
exact null dereferencing point?
It seems to be somewhere in ubi_wl_init, however the patch seems not to
affect these parts of ubi...

Also, what's your CONFIG_MTD_UBI_BEB_LIMIT?

Regards,
Shmulik

Richard Genoud

2012-07-07 08:32:45 UTC

Permalink

Post by Shmulik Ladkani
Many thanks for testing.
Could you please verify the crash only occurs with the patch?
Can you provide the vmlinux matching this oops, so I may analyze the
exact null dereferencing point?
It seems to be somewhere in ubi_wl_init, however the patch seems not to
affect these parts of ubi...
Also, what's your CONFIG_MTD_UBI_BEB_LIMIT?

I'll do that on monday.

Nice week-end !

Richard Genoud

2012-07-09 06:58:28 UTC

Permalink

Hi !
I can't reproduce it...
Maybe the problem was between the chair and the keyboard.
Anyway, if I ran into it again, I'll let you know.

Richard.

Artem Bityutskiy

2012-07-16 15:33:57 UTC

Permalink

Thanks Shmulik - I did not have time to look at the patches, but the
overall description looks good. I will review the patches as soon as I
can. Thanks for making sense of this mess.

But one more think is the mtd web-site. I've grepped for '1%' and there
are plenty of them. I've changed them all to 2% more or less
mechanically - only cleaned up one section by removing out-of-date
information. Would you please grep for '2%' and review if the
information there is reasonable? Also, would you please add some more
info to this FAQ entry:

http://linux-mtd.infradead.org/faq/ubi.html#L_bad_blocks_exceeded

Or even better if you could write a separate section for this stuff in
the documentation, then you could remove that FAQ entry completely.

Thanks a lot!
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120716/0f83ee6d/attachment.sig>

Shmulik Ladkani

2012-07-17 07:23:33 UTC

Permalink

Hi Artem,

Post by Artem Bityutskiy
But one more think is the mtd web-site. I've grepped for '1%' and there
are plenty of them. I've changed them all to 2% more or less
mechanically - only cleaned up one section by removing out-of-date
information. Would you please grep for '2%' and review if the
information there is reasonable? Also, would you please add some more
http://linux-mtd.infradead.org/faq/ubi.html#L_bad_blocks_exceeded
Or even better if you could write a separate section for this stuff in
the documentation, then you could remove that FAQ entry completely.

Sure, I'll try to do it as well.

But it would only make sense after accepting Richard's patchset as well,
which suggests configuring per-ubi-device beb limit via the attach ioctl.

I didn't had the time to properly review it yet, but IMO it looks more
controversial.
http://lists.infradead.org/pipermail/linux-mtd/2012-July/042803.html

Regards,
Shmulik

Artem Bityutskiy

2012-07-18 06:54:21 UTC

Permalink

Post by Shmulik Ladkani
But it would only make sense after accepting Richard's patchset as well,
which suggests configuring per-ubi-device beb limit via the attach ioctl.

Yes, sure, you are right.
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120718/7fa6ef92/attachment.sig>

Artem Bityutskiy

2012-07-30 13:56:50 UTC

Permalink

Hi Shmulik, I've separated out the defconfig changes and pushed patches
1,2, and 3 to the UBI tree (the master branch). Patches 4 and 5 are
already merged upstream. I did a couple of minor modifications in
commentaries and messages and I think in variables declaration section,
nothing else. I'll send you the patches separately.

Thanks!
--
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20120730/d85163aa/attachment.sig>

Shmulik Ladkani

2012-07-31 08:19:22 UTC

Permalink

Hi Artem,

Post by Artem Bityutskiy
Hi Shmulik, I've separated out the defconfig changes and pushed patches
1,2, and 3 to the UBI tree (the master branch). Patches 4 and 5 are
already merged upstream. I did a couple of minor modifications in
commentaries and messages and I think in variables declaration section,
nothing else. I'll send you the patches separately.

Thanks!

I've noticed a diff in the Kconfig describing MTD_UBI_BEB_LIMIT.

In my original [PATCH 2/5] "ubi: Limit amount of reserved eraseblocks
for bad PEB handling" I've amended the MTD_UBI_BEB_LIMIT explanation a
bit.

The diff between what's on linux-ubi and my suggested description is:

- This option specifies the maximum bad physical eraseblocks UBI
- expects on the UBI device (percents of total number of physical
- eraseblocks on this MTD partition). If the underlying flash does not
- admit of bad eraseblocks (e.g. NOR flash), this value is ignored.
+ If the MTD device admits of bad eraseblocks (e.g. NAND flash), UBI
+ reserves some amount of physical eraseblocks to handle new bad
+ eraseblocks.
+ This option specifies the maximum bad eraseblocks UBI expects on the
+ ubi device (percents of total number of flash eraseblocks).
+ This limit is used in order to derive amount of eraseblock UBI
+ reserves for handling new bad blocks.
+ If the device has more bad eraseblocks than this limit, UBI does not
+ reserve any physical eraseblocks for new bad eraseblocks, but
+ attempts to use available eraseblocks (if any).
+ If the underlying flash does not admit of bad eraseblocks (e.g. NOR
+ flash), this value is ignored.

Just wanted to make sure you deliberately discarded these amendments.

Regards,
Shmulik