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

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

[Shmulik Ladkani]

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 patchset changes the mechanism to overcome these flaws.

The caculation of PEBs reserved for bad PEB handling will be based on a
new property, ubi->bad_peb_limit, which specifies an upper limit of PEBs
ubi expects to go bad (currently initialized to a fixed percentage of
total PEBs in the ubi device, configurable via CONFIG_MTD_UBI_BEB_LIMIT,
but can be easily augmented to support a per ubi-attach limit).

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.


Shmulik Ladkani (5):
  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

 arch/arm/configs/sam9_l9260_defconfig |    2 +-
 drivers/mtd/ubi/Kconfig               |   24 ++++++++++-------
 drivers/mtd/ubi/build.c               |   13 +++++++++-
 drivers/mtd/ubi/misc.c                |   44 +++++++++++++++++++++++++++++---
 drivers/mtd/ubi/ubi.h                 |    6 ++--
 drivers/mtd/ubi/vmt.c                 |   20 +-------------
 drivers/mtd/ubi/wl.c                  |   44 +++++++++++++++++++++------------
 7 files changed, 99 insertions(+), 54 deletions(-)

-- 
1.7.9

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

[Shmulik Ladkani]

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 patchset changes the mechanism to overcome these flaws.

The caculation of PEBs reserved for bad PEB handling will be based on a
new property, ubi->bad_peb_limit, which specifies an upper limit of PEBs
ubi expects to go bad (currently initialized to a fixed percentage of
total PEBs in the ubi device, configurable via CONFIG_MTD_UBI_BEB_LIMIT,
but can be easily augmented to support a per ubi-attach limit).

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.


Shmulik Ladkani (5):
  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

 arch/arm/configs/sam9_l9260_defconfig |    2 +-
 drivers/mtd/ubi/Kconfig               |   24 ++++++++++-------
 drivers/mtd/ubi/build.c               |   13 +++++++++-
 drivers/mtd/ubi/misc.c                |   44 +++++++++++++++++++++++++++++---
 drivers/mtd/ubi/ubi.h                 |    6 ++--
 drivers/mtd/ubi/vmt.c                 |   20 +-------------
 drivers/mtd/ubi/wl.c                  |   44 +++++++++++++++++++++------------
 7 files changed, 99 insertions(+), 54 deletions(-)

-- 
1.7.9

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[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@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

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[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@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

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[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@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

[PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

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@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

[PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

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@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

[PATCH 3/5] ubi: kill CONFIG_MTD_UBI_BEB_RESERVE

[Shmulik Ladkani]

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@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

[PATCH 3/5] ubi: kill CONFIG_MTD_UBI_BEB_RESERVE

[Shmulik Ladkani]

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@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

[PATCH 3/5] ubi: kill CONFIG_MTD_UBI_BEB_RESERVE

[Shmulik Ladkani]

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@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

[PATCH 4/5] ubi: trivial: fix comment of ubi_calculate_reserved()

[Shmulik Ladkani]

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

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@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

[PATCH 4/5] ubi: trivial: fix comment of ubi_calculate_reserved()

[Shmulik Ladkani]

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

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@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

[PATCH 5/5] ubi: harmonize the update of ubi->beb_rsvd_pebs

[Shmulik Ladkani]

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@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

[PATCH 5/5] ubi: harmonize the update of ubi->beb_rsvd_pebs

[Shmulik Ladkani]

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@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

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

[Richard Weinberger]

On 04.07.2012 10:05, Shmulik Ladkani wrote:
> Shmulik Ladkani (5):
>    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

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

[Richard Weinberger]

On 04.07.2012 10:05, Shmulik Ladkani wrote:
> Shmulik Ladkani (5):
>    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

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

[Shmulik Ladkani]

On Wed, 04 Jul 2012 10:35:42 +0200 Richard Weinberger <richard@nod.at> wrote:
> Has this patch set an impact on UBI fastmap?

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

> ...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

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

[Shmulik Ladkani]

On Wed, 04 Jul 2012 10:35:42 +0200 Richard Weinberger <richard@nod.at> wrote:
> Has this patch set an impact on UBI fastmap?

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

> ...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

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

[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.

# 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





2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> On Wed, 04 Jul 2012 10:35:42 +0200 Richard Weinberger <richard@nod.at> wrote:
>> Has this patch set an impact on UBI fastmap?
>
> Not sure yet. Maybe. Need to medidate on this :-)
>
>> ...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 ?

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

[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.

# 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





2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> On Wed, 04 Jul 2012 10:35:42 +0200 Richard Weinberger <richard@nod.at> wrote:
>> Has this patch set an impact on UBI fastmap?
>
> Not sure yet. Maybe. Need to medidate on this :-)
>
>> ...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 ?

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

[Shmulik Ladkani]

Hi Richard,

On Fri, 6 Jul 2012 17:27:59 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> 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

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

[Shmulik Ladkani]

Hi Richard,

On Fri, 6 Jul 2012 17:27:59 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> 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

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

[Richard Genoud]

2012/7/7 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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 !

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

[Richard Genoud]

2012/7/7 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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 !

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

[Richard Genoud]

2012/7/7 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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...

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.

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

[Richard Genoud]

2012/7/7 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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...

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.

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Richard Genoud]

2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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[ ?

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Richard Genoud]

2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> 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[ ?

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

On Mon, 9 Jul 2012 12:15:17 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> 2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> > +       /*
> > +        * 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

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

On Mon, 9 Jul 2012 12:15:17 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> 2012/7/4 Shmulik Ladkani <shmulik.ladkani@gmail.com>:
> > +       /*
> > +        * 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

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

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:05 +0300, Shmulik Ladkani wrote:
> 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:05 +0300, Shmulik Ladkani wrote:
> 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Shmulik Ladkani]

Hi Artem,

On Mon, 16 Jul 2012 18:33:57 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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.

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

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

[Shmulik Ladkani]

Hi Artem,

On Mon, 16 Jul 2012 18:33:57 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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.

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

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

[Artem Bityutskiy]

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

On Tue, 2012-07-17 at 10:23 +0300, Shmulik Ladkani wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Artem Bityutskiy]

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

On Tue, 2012-07-17 at 10:23 +0300, Shmulik Ladkani wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@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-arm-kernel/attachments/20120718/fff33c14/attachment-0001.sig>

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> @@ -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.

>  
> @@ -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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> @@ -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.

>  
> @@ -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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/5] ubi: trivial: fix comment of ubi_calculate_reserved()

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> The function name within the comment was not aligned with the actual
> function name.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Pushed this one directly to the master branch, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/5] ubi: trivial: fix comment of ubi_calculate_reserved()

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> The function name within the comment was not aligned with the actual
> function name.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Pushed this one directly to the master branch, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@gmail.com>
> ---
>  arch/arm/configs/sam9_l9260_defconfig |    1 +

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@gmail.com>
> ---
>  arch/arm/configs/sam9_l9260_defconfig |    1 +

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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@gmail.com>
> ---
>  arch/arm/configs/sam9_l9260_defconfig |    1 +

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-arm-kernel/attachments/20120718/544983b4/attachment.sig>

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-18 at 13:28 +0300, Artem Bityutskiy wrote:
> >       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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-18 at 13:28 +0300, Artem Bityutskiy wrote:
> >       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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/5] ubi: harmonize the update of ubi->beb_rsvd_pebs

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> +       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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 5/5] ubi: harmonize the update of ubi->beb_rsvd_pebs

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> +       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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:06 +0300, Shmulik Ladkani wrote:
> 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

Hi Artem,

On Wed, 18 Jul 2012 13:28:37 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Shmulik Ladkani]

Hi Artem,

On Wed, 18 Jul 2012 13:28:37 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-18 at 22:55 +0300, Shmulik Ladkani wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] ubi: Limit amount of reserved eraseblocks for bad PEB handling

[Artem Bityutskiy]

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

On Wed, 2012-07-18 at 22:55 +0300, Shmulik Ladkani wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Shmulik Ladkani]

On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Shmulik Ladkani]

On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Shmulik Ladkani]

On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Thu, 2012-07-19 at 09:16 +0300, Shmulik Ladkani wrote:
> On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

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

On Thu, 2012-07-19 at 09:16 +0300, Shmulik Ladkani wrote:
> On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/5] ubi: introduce ubi->bad_peb_limit

[Artem Bityutskiy]

On Thu, 2012-07-19 at 09:16 +0300, Shmulik Ladkani wrote:
> On Wed, 18 Jul 2012 13:40:53 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > 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-arm-kernel/attachments/20120730/a9d45a61/attachment.sig>

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

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:05 +0300, Shmulik Ladkani wrote:
> 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 patchset changes the mechanism to overcome these flaws.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Artem Bityutskiy]

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

On Wed, 2012-07-04 at 11:05 +0300, Shmulik Ladkani wrote:
> 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 patchset changes the mechanism to overcome these flaws.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Shmulik Ladkani]

Hi Artem,

On Mon, 30 Jul 2012 16:56:50 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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

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

[Shmulik Ladkani]

Hi Artem,

On Mon, 30 Jul 2012 16:56:50 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 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