Message ID | 1345043331.3393.151.camel@sauron.fi.intel.com |
---|---|
State | New, archived |
Headers | show |
2012/8/15 Artem Bityutskiy <dedekind1@gmail.com>: > On Tue, 2012-07-10 at 18:23 +0200, Richard Genoud wrote: >> + /* we are using here the whole MTD device size and not >> + * the MTD partition size because the maximum number of >> + * bad blocks is a percentage of the whole device and >> + * the bad blocks are not fairly disposed on a flash >> + * device >> + */ > > Would you please use proper kernel-style comments instead, to be > consistent with the rest of the UBI code? I've amended this one, but > wanted to note for future. ok, sorry for that. > > I've re-based your patch against the latest UBI. I've also tried to > improve the Kconfig help message as well. Below is the patch I ended up > with. > > > From cb14c6c5455443cbe960a36e77b3fcd0e5bc7152 Mon Sep 17 00:00:00 2001 > From: Richard Genoud <richard.genoud@gmail.com> > Date: Tue, 10 Jul 2012 18:23:41 +0200 > Subject: [PATCH 2/2] UBI: use the whole MTD device size to get bad_peb_limit > > On NAND flash devices, UBI reserves some physical erase blocks (PEB) for > bad block handling. Today, the number of reserved PEB can only be set as a > percentage of the total number of PEB in each MTD partition. For example, for a > NAND flash with 128KiB PEB, 2 MTD partition of 20MiB (mtd0) and 100MiB (mtd1) > and 2% reserved PEB: > - the UBI device on mtd0 will have 2 PEB reserved > - the UBI device on mtd1 will have 16 PEB reserved > > The problem with this behaviour is that NAND flash manufacturers give a > minimum number of valid block (NVB) during the endurance life of the > device, e.g.: > > Parameter Symbol Min Max Unit Notes > -------------------------------------------------------------- > Valid block number NVB 1004 1024 Blocks 1 > Note: > 1. Invalid blocks are block that contain one or more bad bits beyond > ECC. The device may contain bad blocks upon shipment. Additional bad > blocks may develop over time; however, the total number of available > blocks will not drop below NVB during the endurance life of the device. > > From this number we can deduce the maximum number of bad PEB that a device will > contain during its endurance life: a 128MiB NAND flash (1024 PEB) will not have > less than 20 bad blocks during the flash endurance life. > > But the manufacturer doesn't tell where those bad block will appear. He doesn't > say either if they will be equally disposed on the whole device (and I'm pretty > sure they won't). So, according to the datasheets, we should reserve the > maximum number of bad PEB for each UBI device (worst case scenario: 20 bad > blocks appears on the smallest MTD partition). > > So this patch make UBI use the whole MTD device size to calculate the maximum > bad expected eraseblocks. > > The Kconfig option is in per1024 blocks, thus it can have a default value of 20 > which is *very* common for NAND devices. > > Signed-off-by: Richard Genoud <richard.genoud@gmail.com> > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > --- > drivers/mtd/ubi/Kconfig | 27 +++++++++++++++++++++------ > drivers/mtd/ubi/build.c | 21 ++++++++++++++++++--- > 2 files changed, 39 insertions(+), 9 deletions(-) > > diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig > index b2f4f0f..98bda6c 100644 > --- a/drivers/mtd/ubi/Kconfig > +++ b/drivers/mtd/ubi/Kconfig > @@ -28,14 +28,29 @@ config MTD_UBI_WL_THRESHOLD > to 128 or 256, although it does not have to be power of 2). > > config MTD_UBI_BEB_LIMIT > - int "Percentage of maximum expected bad eraseblocks" > - default 2 > - range 0 25 > + int "Maximum expected bad eraseblock count per 1024 eraseblocks" > + default 20 > + range 2 256 > help > 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. > + expects on the MTD device (per 1024 eraseblocks). If the underlying > + flash does not admit of bad eraseblocks (e.g. NOR flash), this value > + is ignored. > + > + NAND datasheets often specify the minimum and maximum NVM (Number of > + Valid Blocks) for the flashes' endurance lifetime. The maximum > + expected bad eraseblocks per 1024 eraseblocks then can be calculated > + as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs > + (MaxNVB is basically the total count of eraseblocks on the chip). > + > + To put it differently, if this value is 20, UBI will try to reserve > + about 1.9% of physical eraseblocks for bad blocks handling. And that > + will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD > + partition UBI attaches. This means that if you have, say, if a NAND I don't quite understand this sentence. Maybe you meant: This means that if you have, say, a NAND flash chip that admits a maximum of 40 bad eraseblocks [...] (but I'm not a native english speaker !) > + flash chip admits maximum 40 bad eraseblocks, and it is split on two > + MTD partitions of the same size, UBI will reserve 40 eraseblocks when > + attaching a partition. > + > Leave the default value if unsure. > Best regards, Richard.
Hi Artem, Richard, On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: > 1. Invalid blocks are block that contain one or more bad bits beyond > ECC. I would remove this one sentence from the log, it is misleading; invalid blocks are not necessarily related to ECC. > if (CONFIG_MTD_UBI_BEB_LIMIT > 0) { > - int percent = CONFIG_MTD_UBI_BEB_LIMIT; > - int limit = mult_frac(ubi->peb_count, percent, 100); > + int per1024 = CONFIG_MTD_UBI_BEB_LIMIT; > + int limit, device_pebs; > + uint64_t device_size; > + > + /* > + * Here we are using size of the entire flash chip and > + * not just the MTD partition size because the maximum > + * number of bad eraseblocks is a percentage of the > + * whole device and bad eraseblocks are not fairly > + * distributed over the flash chip. So the worst case > + * is that all the bad eraseblocks of the chip are in > + * the MTD partition we are attaching (ubi->mtd). > + */ > + device_size = mtd_get_device_size(ubi->mtd); > + device_pebs = mtd_div_by_eb(device_size, ubi->mtd); > + limit = mult_frac(device_pebs, per1024, 1024); > > /* Round it up */ > - if (mult_frac(limit, 100, percent) < ubi->peb_count) > + if (mult_frac(limit, 1024, per1024) < ubi->peb_count) Oops... should be: + if (mult_frac(limit, 1024, per1024) < device_pebs) Regards, Shmulik
Hi, One more thing... On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: > config MTD_UBI_BEB_LIMIT > - int "Percentage of maximum expected bad eraseblocks" > - default 2 > - range 0 25 > + int "Maximum expected bad eraseblock count per 1024 eraseblocks" > + default 20 > + range 2 256 Those defconfigs that explicilty set an original LIMIT should be adjusted as well, as the units have changed, no? Regards, Shmulik
2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>: > Hi Artem, Richard, > > On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: >> 1. Invalid blocks are block that contain one or more bad bits beyond >> ECC. > > I would remove this one sentence from the log, it is misleading; invalid > blocks are not necessarily related to ECC. I agree (even if this sentence is from a nand datasheet !) > >> if (CONFIG_MTD_UBI_BEB_LIMIT > 0) { >> - int percent = CONFIG_MTD_UBI_BEB_LIMIT; >> - int limit = mult_frac(ubi->peb_count, percent, 100); >> + int per1024 = CONFIG_MTD_UBI_BEB_LIMIT; >> + int limit, device_pebs; >> + uint64_t device_size; >> + >> + /* >> + * Here we are using size of the entire flash chip and >> + * not just the MTD partition size because the maximum >> + * number of bad eraseblocks is a percentage of the >> + * whole device and bad eraseblocks are not fairly >> + * distributed over the flash chip. So the worst case >> + * is that all the bad eraseblocks of the chip are in >> + * the MTD partition we are attaching (ubi->mtd). >> + */ >> + device_size = mtd_get_device_size(ubi->mtd); >> + device_pebs = mtd_div_by_eb(device_size, ubi->mtd); >> + limit = mult_frac(device_pebs, per1024, 1024); >> >> /* Round it up */ >> - if (mult_frac(limit, 100, percent) < ubi->peb_count) >> + if (mult_frac(limit, 1024, per1024) < ubi->peb_count) > > Oops... should be: > > + if (mult_frac(limit, 1024, per1024) < device_pebs) > > Regards, > Shmulik Ok, I'll change that, may be in a separate patch, as it's a bug fix. I'll add your signoff. thanks !
On Thu, 2012-08-16 at 11:25 +0300, Shmulik Ladkani wrote: > I would remove this one sentence from the log, it is misleading; invalid > blocks are not necessarily related to ECC. Done. > + if (mult_frac(limit, 1024, per1024) < device_pebs) Done, thanks!
On Thu, 2012-08-16 at 12:35 +0200, Richard Genoud wrote:
> Ok, I'll change that, may be in a separate patch, as it's a bug fix.
Let me take care of this patch. I'll amend it myself, push to my tree
and re-send to the mailing list for your review. Please, provide an
updated version of the other patches instead.
P.S. Of course, your authorship will be preserved.
On Thu, 2012-08-16 at 11:32 +0300, Shmulik Ladkani wrote: > > - default 2 > > - range 0 25 > > + int "Maximum expected bad eraseblock count per 1024 eraseblocks" > > + default 20 > > + range 2 256 > > Those defconfigs that explicilty set an original LIMIT should be > adjusted as well, as the units have changed, no? Yes, I'll do this, thanks!
2012/8/16 Shmulik Ladkani <shmulik.ladkani@gmail.com>: > Hi, > > One more thing... > > On Wed, 15 Aug 2012 18:08:51 +0300 Artem Bityutskiy <dedekind1@gmail.com> wrote: >> config MTD_UBI_BEB_LIMIT >> - int "Percentage of maximum expected bad eraseblocks" >> - default 2 >> - range 0 25 >> + int "Maximum expected bad eraseblock count per 1024 eraseblocks" >> + default 20 >> + range 2 256 > > Those defconfigs that explicilty set an original LIMIT should be > adjusted as well, as the units have changed, no? you mean that it should be between 0 and 256 ?
On Thu, 2012-08-16 at 10:13 +0200, Richard Genoud wrote: > > + To put it differently, if this value is 20, UBI will try > to reserve > > + about 1.9% of physical eraseblocks for bad blocks > handling. And that > > + will be 1.9% of eraseblocks on the entire NAND chip, not > just the MTD > > + partition UBI attaches. This means that if you have, say, > if a NAND > I don't quite understand this sentence. > Maybe you meant: > This means that if you have, say, a NAND flash chip that admits a > maximum of 40 bad eraseblocks [...] > (but I'm not a native english speaker !) Fixed, thanks!
diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig index b2f4f0f..98bda6c 100644 --- a/drivers/mtd/ubi/Kconfig +++ b/drivers/mtd/ubi/Kconfig @@ -28,14 +28,29 @@ config MTD_UBI_WL_THRESHOLD to 128 or 256, although it does not have to be power of 2). config MTD_UBI_BEB_LIMIT - int "Percentage of maximum expected bad eraseblocks" - default 2 - range 0 25 + int "Maximum expected bad eraseblock count per 1024 eraseblocks" + default 20 + range 2 256 help 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. + expects on the MTD device (per 1024 eraseblocks). If the underlying + flash does not admit of bad eraseblocks (e.g. NOR flash), this value + is ignored. + + NAND datasheets often specify the minimum and maximum NVM (Number of + Valid Blocks) for the flashes' endurance lifetime. The maximum + expected bad eraseblocks per 1024 eraseblocks then can be calculated + as "1024 * (1 - MinNVB / MaxNVB)", which gives 20 for most NANDs + (MaxNVB is basically the total count of eraseblocks on the chip). + + To put it differently, if this value is 20, UBI will try to reserve + about 1.9% of physical eraseblocks for bad blocks handling. And that + will be 1.9% of eraseblocks on the entire NAND chip, not just the MTD + partition UBI attaches. This means that if you have, say, if a NAND + flash chip admits maximum 40 bad eraseblocks, and it is split on two + MTD partitions of the same size, UBI will reserve 40 eraseblocks when + attaching a partition. + Leave the default value if unsure. config MTD_UBI_GLUEBI diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index 7b6b5f9..9fd8d86 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -36,6 +36,7 @@ #include <linux/namei.h> #include <linux/stat.h> #include <linux/miscdevice.h> +#include <linux/mtd/partitions.h> #include <linux/log2.h> #include <linux/kthread.h> #include <linux/kernel.h> @@ -610,11 +611,25 @@ static int io_init(struct ubi_device *ubi) 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 limit = mult_frac(ubi->peb_count, percent, 100); + int per1024 = CONFIG_MTD_UBI_BEB_LIMIT; + int limit, device_pebs; + uint64_t device_size; + + /* + * Here we are using size of the entire flash chip and + * not just the MTD partition size because the maximum + * number of bad eraseblocks is a percentage of the + * whole device and bad eraseblocks are not fairly + * distributed over the flash chip. So the worst case + * is that all the bad eraseblocks of the chip are in + * the MTD partition we are attaching (ubi->mtd). + */ + device_size = mtd_get_device_size(ubi->mtd); + device_pebs = mtd_div_by_eb(device_size, ubi->mtd); + limit = mult_frac(device_pebs, per1024, 1024); /* Round it up */ - if (mult_frac(limit, 100, percent) < ubi->peb_count) + if (mult_frac(limit, 1024, per1024) < ubi->peb_count) limit += 1; ubi->bad_peb_limit = limit; }