This is the next round of UBI fastmap updates.
Fastmap is now disabled by default.
If you attach an image it will not automatically convert it
to the fastmap format.
UBI has a new module parameter "fm_auto".
If set to 1 UBI will create a fastmap automatically on your
flash device.
Please see commit "Add a module parameter to enable fastmap"
for more details.
If you want to test fastmap you can use my git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v15
Enjoy!
//richard
[PATCH 01/11] UBI: Fastmap: Fix mem leak in error path
[PATCH 02/11] UBI: Fastmap: Amend self_check_eba()
[PATCH 03/11] UBI: Fastmap: Remove forward declaration
[PATCH 04/11] UBI: Fastmap: Move fastmap specific code into
[PATCH 05/11] UBI: Fastmap: Kill TODO
[PATCH 06/11] UBI: Fastmap: Remove unused variable
[PATCH 07/11] UBI: Fastmap: Fix scan regression
[PATCH 08/11] ubi: fastmap: Remove 'ubi' parameter of 'destroy_ai()'
[PATCH 09/11] ubi: fastmap: Do not free 'ai' in 'scan_all()'
[PATCH 10/11] UBI: Fastmap: Disable fastmap per default
[PATCH 11/11] UBI: Fastmap: Add a module parameter to enable fastmap
Don't BUG() on error and document return values.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 5 ++++-
drivers/mtd/ubi/eba.c | 7 ++++---
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index bd3a8f5..f1dfb63 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1333,8 +1333,11 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
goto out_wl;
}
- self_check_eba(ubi, ai, scan_ai);
+ err = self_check_eba(ubi, ai, scan_ai);
destroy_ai(ubi, scan_ai);
+
+ if (err)
+ goto out_wl;
}
destroy_ai(ubi, ai);
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 6d6e301..4fb80d0 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -1223,7 +1223,9 @@ static void print_rsvd_warning(struct ubi_device *ubi,
* @ai_fastmap: UBI attach info object created by fastmap
* @ai_scan: UBI attach info object created by scanning
*
- * TODO: what we do and what return.
+ * Returns < 0 in case of an internal error, 0 otherwise.
+ * If a bad EBA table entry was found it will be printed out and
+ * ubi_assert() triggers.
*/
int self_check_eba(struct ubi_device *ubi, struct ubi_attach_info *ai_fastmap,
struct ubi_attach_info *ai_scan)
@@ -1291,8 +1293,7 @@ int self_check_eba(struct ubi_device *ubi, struct ubi_attach_info *ai_fastmap,
ubi_err("LEB:%i:%i is PEB:%i instead of %i!",
vol->vol_id, i, fm_eba[i][j], scan_eba[i][j]);
- /* TODO: no, please, return error instead */
- BUG();
+ ubi_assert(0);
}
}
}
--
1.7.6.5
From: Shmulik Ladkani <[email protected]>
The 'ubi' argument of 'destroy_ai' was added during fastmap development,
but it is no longer used.
Signed-off-by: Shmulik Ladkani <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index bc1e4bf..c30a0f6 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1164,10 +1164,9 @@ static void destroy_av(struct ubi_attach_info *ai, struct ubi_ainf_volume *av)
/**
* destroy_ai - destroy attaching information.
- * @ubi: UBI device object
* @ai: attaching information
*/
-static void destroy_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)
+static void destroy_ai(struct ubi_attach_info *ai)
{
struct ubi_ainf_peb *aeb, *aeb_tmp;
struct ubi_ainf_volume *av;
@@ -1300,7 +1299,7 @@ out_vidh:
out_ech:
kfree(ech);
out_ai:
- destroy_ai(ubi, ai);
+ destroy_ai(ai);
return err;
}
@@ -1353,7 +1352,7 @@ out_vidh:
out_ech:
kfree(ech);
out_ai:
- destroy_ai(ubi, ai);
+ destroy_ai(ai);
return err;
}
static struct ubi_attach_info *alloc_ai(void)
@@ -1403,7 +1402,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
err = scan_fast(ubi, ai);
if (err > 0) {
if (err != UBI_NO_FASTMAP) {
- destroy_ai(ubi, ai);
+ destroy_ai(ai);
ai = alloc_ai();
if (!ai)
return -ENOMEM;
@@ -1449,13 +1448,13 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
}
err = self_check_eba(ubi, ai, scan_ai);
- destroy_ai(ubi, scan_ai);
+ destroy_ai(scan_ai);
if (err)
goto out_wl;
}
- destroy_ai(ubi, ai);
+ destroy_ai(ai);
return 0;
out_wl:
@@ -1464,7 +1463,7 @@ out_vtbl:
ubi_free_internal_volumes(ubi);
vfree(ubi->vtbl);
out_ai:
- destroy_ai(ubi, ai);
+ destroy_ai(ai);
return err;
}
--
1.7.6.5
From: Shmulik Ladkani <[email protected]>
Do not call 'destroy_ai(ai)' at error handling of 'scan_all', since:
- 'ai' is allocated in 'ubi_attach' (the caller of scan_all) and
provided as an argument. It's not scan_all's responsibility to free it
- It is not consistent with scan_all's sister function
'ubi_attach_fastmap()' which does not free the given 'ai'
- It will cause a double free as 'ubi_attach' (the caller of scan_all)
already destroys 'ai' in case of an error
Signed-off-by: Shmulik Ladkani <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index c30a0f6..a343a41 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1237,7 +1237,7 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai, int star
ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
if (!ech)
- goto out_ai;
+ return err;
vidh = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
if (!vidh)
@@ -1298,8 +1298,6 @@ out_vidh:
ubi_free_vid_hdr(ubi, vidh);
out_ech:
kfree(ech);
-out_ai:
- destroy_ai(ai);
return err;
}
--
1.7.6.5
calling late_analysis() when fastmap is used does not make sense.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 1b62d54..9878cc2 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1258,7 +1258,6 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai)
if (ai->ec_count)
ai->mean_ec = div_u64(ai->ec_sum, ai->ec_count);
- /* TODO: if we attach by fastmap, we do not execute this? */
err = late_analysis(ubi, ai);
if (err)
goto out_vidh;
--
1.7.6.5
... resuse scan_peb(), don't rescan the first 64 PEBs.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 134 ++++++++++++++++++++++++++++++---------------
drivers/mtd/ubi/fastmap.c | 64 ++--------------------
drivers/mtd/ubi/ubi.h | 2 +-
3 files changed, 95 insertions(+), 105 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 9878cc2..bc1e4bf 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -817,10 +817,11 @@ out_unlock:
* successfully handled and a negative error code in case of failure.
*/
static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,
- int pnum)
+ int pnum, int *vid, unsigned long long *sqnum,
+ int find_fastmap)
{
long long uninitialized_var(ec);
- int err, bitflips = 0, vol_id, ec_err = 0;
+ int err, bitflips = 0, vol_id = -1, ec_err = 0;
dbg_bld("scan PEB %d", pnum);
@@ -991,8 +992,13 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,
}
vol_id = be32_to_cpu(vidh->vol_id);
+ if (vid)
+ *vid = vol_id;
+ if (sqnum)
+ *sqnum = be64_to_cpu(vidh->sqnum);
- if (vol_id > UBI_MAX_VOLUMES && vol_id != UBI_LAYOUT_VOLUME_ID) {
+ if (!find_fastmap &&
+ (vol_id > UBI_MAX_VOLUMES && vol_id != UBI_LAYOUT_VOLUME_ID)) {
int lnum = be32_to_cpu(vidh->lnum);
/* Unsupported internal volume */
@@ -1221,7 +1227,7 @@ static void destroy_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)
* information about it in form of a "struct ubi_attach_info" object. In case
* of failure, an error code is returned.
*/
-static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai)
+static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai, int start)
{
int err, pnum;
struct rb_node *rb1, *rb2;
@@ -1229,11 +1235,6 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai)
struct ubi_ainf_peb *aeb;
err = -ENOMEM;
- ai->aeb_slab_cache = kmem_cache_create("ubi_aeb_slab_cache",
- sizeof(struct ubi_ainf_peb),
- 0, 0, NULL);
- if (!ai->aeb_slab_cache)
- goto out_ai;
ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
if (!ech)
@@ -1243,11 +1244,11 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai)
if (!vidh)
goto out_ech;
- for (pnum = 0; pnum < ubi->peb_count; pnum++) {
+ for (pnum = start; pnum < ubi->peb_count; pnum++) {
cond_resched();
dbg_gen("process PEB %d", pnum);
- err = scan_peb(ubi, ai, pnum);
+ err = scan_peb(ubi, ai, pnum, NULL, NULL, 0);
if (err < 0)
goto out_vidh;
}
@@ -1303,17 +1304,77 @@ out_ai:
return err;
}
+/**
+ * scan_fastmap - try to find a fastmap and attach from it.
+ * @ubi: UBI device description object
+ * @ai: attach info object
+ */
+static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
+{
+ int err, pnum, fm_anchor = -1;
+ unsigned long long max_sqnum = 0;
+
+ err = -ENOMEM;
+
+ ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
+ if (!ech)
+ goto out_ai;
+
+ vidh = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
+ if (!vidh)
+ goto out_ech;
+
+ for (pnum = 0; pnum < UBI_FM_MAX_START; pnum++) {
+ int vol_id = -1;
+ unsigned long long sqnum = -1;
+ cond_resched();
+
+ dbg_gen("process PEB %d", pnum);
+ err = scan_peb(ubi, ai, pnum, &vol_id, &sqnum, 1);
+ if (err < 0)
+ goto out_vidh;
+
+ if (vol_id == UBI_FM_SB_VOLUME_ID && sqnum > max_sqnum) {
+ max_sqnum = sqnum;
+ fm_anchor = pnum;
+ }
+ }
+
+ ubi_free_vid_hdr(ubi, vidh);
+ kfree(ech);
+
+ if (fm_anchor < 0)
+ return UBI_NO_FASTMAP;
+
+ return ubi_scan_fastmap(ubi, ai, fm_anchor);
+
+out_vidh:
+ ubi_free_vid_hdr(ubi, vidh);
+out_ech:
+ kfree(ech);
+out_ai:
+ destroy_ai(ubi, ai);
+ return err;
+}
static struct ubi_attach_info *alloc_ai(void)
{
- static struct ubi_attach_info *ai;
+ struct ubi_attach_info *ai;
ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
- if (ai) {
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
+ if (!ai)
+ return ai;
+
+ INIT_LIST_HEAD(&ai->corr);
+ INIT_LIST_HEAD(&ai->free);
+ INIT_LIST_HEAD(&ai->erase);
+ INIT_LIST_HEAD(&ai->alien);
+ ai->volumes = RB_ROOT;
+ ai->aeb_slab_cache = kmem_cache_create("ubi_aeb_slab_cache",
+ sizeof(struct ubi_ainf_peb),
+ 0, 0, NULL);
+ if (!ai->aeb_slab_cache) {
+ kfree(ai);
+ ai = NULL;
}
return ai;
@@ -1337,35 +1398,18 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
return -ENOMEM;
if (force_scan)
- err = scan_all(ubi, ai);
+ err = scan_all(ubi, ai, 0);
else {
- /* TODO: this is a regression. If I have an old image, and I do
- * not want to use fastmap, I will be forced to waste time for
- * useless scan of 64 first eraseblocks. Not good.
- *
- * Can you teach ubi_scan_fastmap() to use 'scan_peb()'
- * function for scanning and build normal ai information? If it
- * finds fastmap - it can destroy the collected ai. If it does
- * not find, it returns ai. Then you just confinue scanning.
- *
- * I buess what we'll need is:
- * 1. scan_all() -> scan_range(..., int pnum1, int pnum2);
- * 2. ubi_scan_fastmap() returns the pnum of the last scanned
- * eraseblock if fastmap was not found;
- * Also 'ubi_scan_fastmap()' uses scan_peb() for scanning.
- * 3. You call 'scan_range(..., pnum, c->peb_cnt - 1)' and
- * it continues.
- *
- * And no regressions.
- */
- err = ubi_scan_fastmap(ubi, ai);
+ err = scan_fast(ubi, ai);
if (err > 0) {
- destroy_ai(ubi, ai);
- ai = alloc_ai();
- if (!ai)
- return -ENOMEM;
+ if (err != UBI_NO_FASTMAP) {
+ destroy_ai(ubi, ai);
+ ai = alloc_ai();
+ if (!ai)
+ return -ENOMEM;
+ }
- err = scan_all(ubi, ai);
+ err = scan_all(ubi, ai, UBI_FM_MAX_START);
}
}
@@ -1398,7 +1442,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
if (!scan_ai)
goto out_wl;
- err = scan_all(ubi, scan_ai);
+ err = scan_all(ubi, scan_ai, 0);
if (err) {
kfree(scan_ai);
goto out_wl;
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 9b4fd34..6276039 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -819,82 +819,28 @@ fail:
}
/**
- * ubi_find_fastmap - searches the first UBI_FM_MAX_START PEBs for the
- * fastmap super block.
- * @ubi: UBI device object
- * @fm_start: Pointer where the fastmap suber block PEB number will be stored.
- *
- * Returns:
- * - 0 on success: (fm_start contains suber block PEB number)
- * - < 0 on failure (fm_start is -1)
- */
-static int ubi_find_fastmap(struct ubi_device *ubi, int *fm_start)
-{
- int i, ret = -ENOENT;
- struct ubi_vid_hdr *vhdr;
- unsigned long long max_sqnum = 0, sqnum;
-
- vhdr = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
- if (!vhdr)
- return -ENOMEM;
-
- *fm_start = -1;
- for (i = 0; i < UBI_FM_MAX_START; i++) {
- if (ubi_io_is_bad(ubi, i))
- continue;
-
- ret = ubi_io_read_vid_hdr(ubi, i, vhdr, 0);
- if (ret < 0)
- goto out;
- else if (ret > 0 && ret != UBI_IO_BITFLIPS)
- continue;
-
- if (be32_to_cpu(vhdr->vol_id) == UBI_FM_SB_VOLUME_ID) {
- sqnum = be64_to_cpu(vhdr->sqnum);
- dbg_bld("found a fastmap super block at PEB %i " \
- "sqnum: %llu", i, sqnum);
-
- if (sqnum > max_sqnum) {
- max_sqnum = sqnum;
- *fm_start = i;
- }
- }
- }
-
- if (*fm_start > -1)
- ret = 0;
-out:
- ubi_free_vid_hdr(ubi, vhdr);
- return ret;
-}
-
-/**
* ubi_scan_fastmap - scan the fastmap.
* @ubi: UBI device object
* @ai: UBI attach info to be filled
+ * @fm_anchor: The fastmap starts at this PEB
*
* Returns 0 on success, UBI_NO_FASTMAP if no fastmap was found,
* UBI_BAD_FASTMAP if one was found but is not usable.
* < 0 indicates an internal error.
*/
-int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai)
+int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
+ int fm_anchor)
{
struct ubi_fm_sb *fmsb;
struct ubi_vid_hdr *vh;
struct ubi_ec_hdr *ech;
struct ubi_fastmap_layout *fm;
- int i, used_blocks, pnum, sb_pnum = 0, ret = 0;
+ int i, used_blocks, pnum, ret = 0;
void *fm_raw = NULL;
size_t fm_size;
__be32 crc, tmp_crc;
unsigned long long sqnum = 0;
- ret = ubi_find_fastmap(ubi, &sb_pnum);
- if (ret)
- return ret;
- if (sb_pnum == -1)
- return UBI_NO_FASTMAP;
-
fmsb = kmalloc(sizeof(*fmsb), GFP_KERNEL);
if (!fmsb) {
ret = -ENOMEM;
@@ -908,7 +854,7 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai)
goto free_raw;
}
- ret = ubi_io_read(ubi, fmsb, sb_pnum, ubi->leb_start, sizeof(*fmsb));
+ ret = ubi_io_read(ubi, fmsb, fm_anchor, ubi->leb_start, sizeof(*fmsb));
if (ret && ret != UBI_IO_BITFLIPS) {
kfree(fmsb);
kfree(fm);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 8e2592d..fef9e92 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -816,7 +816,7 @@ int ubi_compare_lebs(struct ubi_device *ubi, const struct ubi_ainf_peb *aeb,
/* fastmap.c */
int ubi_update_fastmap(struct ubi_device *ubi);
-int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai);
+int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai, int fm_anchor);
/*
* ubi_rb_for_each_entry - walk an RB-tree.
--
1.7.6.5
The new parameter, ubi.fm_auto is per default 0.
If you attach an old image without a fastmap installed
UBI will not install a fastmap an work like in the old days.
But attaching by fastmap will work too if you attach a new image.
Of course in this case the fastmap will also get updated.
Is ubi.fm_auto set to 1 UBI automatically installs a fastmap
on old images.
This can be used to convert old images to have fastmap support.
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 2 +-
drivers/mtd/ubi/build.c | 8 +++++++-
drivers/mtd/ubi/fastmap.c | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index c55ad0f..a343a41 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1394,7 +1394,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
if (!ai)
return -ENOMEM;
- if (force_scan || ubi->fm_disabled)
+ if (force_scan)
err = scan_all(ubi, ai, 0);
else {
err = scan_fast(ubi, ai);
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 7b5dc5d..50b7590 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -67,6 +67,8 @@ static int __initdata mtd_devs;
/* MTD devices specification parameters */
static struct mtd_dev_param __initdata mtd_dev_param[UBI_MAX_DEVICES];
+/* UBI module parameter to enable fastmap automatically on non-fastmap images */
+static bool fm_auto;
/* Root UBI "class" object (corresponds to '/<sysfs>/class/ubi/') */
struct class *ubi_class;
@@ -899,7 +901,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
ubi->fm_pool.max_size = UBI_FM_MIN_POOL_SIZE;
ubi->fm_wl_pool.max_size = UBI_FM_WL_POOL_SIZE;
- ubi->fm_disabled = 1;
+ ubi->fm_disabled = !fm_auto;
ubi_msg("default fastmap pool size: %d", ubi->fm_pool.max_size);
ubi_msg("default fastmap WL pool size: %d", ubi->fm_wl_pool.max_size);
@@ -1392,6 +1394,10 @@ MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: "
"with name \"content\" using VID header offset 1984, and "
"MTD device number 4 with default VID header offset.");
+module_param(fm_auto, bool, 000);
+MODULE_PARM_DESC(fm_auto, "Set this parameter to enable fastmap automatically "
+ "on images without a fastmap.");
+
MODULE_VERSION(__stringify(UBI_VERSION));
MODULE_DESCRIPTION("UBI - Unsorted Block Images");
MODULE_AUTHOR("Artem Bityutskiy");
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 0c9466d..d995105 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -1044,8 +1044,10 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai,
ubi->fm = fm;
ubi->fm_pool.max_size = ubi->fm->max_pool_size;
ubi->fm_wl_pool.max_size = ubi->fm->max_wl_pool_size;
+ ubi_msg("attached by fastmap");
ubi_msg("fastmap pool size: %d", ubi->fm_pool.max_size);
ubi_msg("fastmap WL pool size: %d", ubi->fm_wl_pool.max_size);
+ ubi->fm_disabled = 0;
free_hdr:
ubi_free_vid_hdr(ubi, vh);
--
1.7.6.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 2 +-
drivers/mtd/ubi/build.c | 1 +
drivers/mtd/ubi/fastmap.c | 2 +-
drivers/mtd/ubi/ubi.h | 2 ++
drivers/mtd/ubi/wl.c | 5 +++--
5 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index a343a41..c55ad0f 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1394,7 +1394,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
if (!ai)
return -ENOMEM;
- if (force_scan)
+ if (force_scan || ubi->fm_disabled)
err = scan_all(ubi, ai, 0);
else {
err = scan_fast(ubi, ai);
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 7094550..7b5dc5d 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -899,6 +899,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
ubi->fm_pool.max_size = UBI_FM_MIN_POOL_SIZE;
ubi->fm_wl_pool.max_size = UBI_FM_WL_POOL_SIZE;
+ ubi->fm_disabled = 1;
ubi_msg("default fastmap pool size: %d", ubi->fm_pool.max_size);
ubi_msg("default fastmap WL pool size: %d", ubi->fm_wl_pool.max_size);
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 6276039..0c9466d 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -1392,7 +1392,7 @@ int ubi_update_fastmap(struct ubi_device *ubi)
ubi_refill_pools(ubi);
- if (ubi->ro_mode) {
+ if (ubi->ro_mode || ubi->fm_disabled) {
mutex_unlock(&ubi->fm_mutex);
return 0;
}
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index fef9e92..9f766ff 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -386,6 +386,7 @@ struct ubi_wl_entry;
* @ltree: the lock tree
* @alc_mutex: serializes "atomic LEB change" operations
*
+ * @fm_disabled: non-zero if fastmap is disabled (default)
* @fm: in-memory data structure of the currently used fastmap
* @fm_pool: in-memory data structure of the fastmap pool
* @fm_wl_pool: in-memory data structure of the fastmap pool used by the WL
@@ -486,6 +487,7 @@ struct ubi_device {
struct mutex alc_mutex;
/* Fastmap stuff */
+ int fm_disabled;
struct ubi_fastmap_layout *fm;
struct ubi_fm_pool fm_pool;
struct ubi_fm_pool fm_wl_pool;
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 28385d2..6c69017 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -376,7 +376,7 @@ static struct ubi_wl_entry *find_wl_entry(struct ubi_device *ubi,
/* If no fastmap has been written and this WL entry can be used
* as anchor PEB, hold it back and return the second best WL entry
* such that fastmap can use the anchor PEB later. */
- if (!ubi->fm && e->pnum < UBI_FM_MAX_START)
+ if (!ubi->fm_disabled && !ubi->fm && e->pnum < UBI_FM_MAX_START)
return prev_e;
return e;
@@ -405,7 +405,8 @@ static struct ubi_wl_entry *find_mean_wl_entry(struct ubi_device *ubi,
/* If no fastmap has been written and this WL entry can be used
* as anchor PEB, hold it back and return the second best
* WL entry such that fastmap can use the anchor PEB later. */
- if (e && !ubi->fm && e->pnum < UBI_FM_MAX_START)
+ if (e && !ubi->fm_disabled && !ubi->fm &&
+ e->pnum < UBI_FM_MAX_START)
e = rb_entry(rb_next(root->rb_node),
struct ubi_wl_entry, u.rb);
} else
--
1.7.6.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/ubi.h | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 6bf5349..8e2592d 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -393,8 +393,6 @@ struct ubi_wl_entry;
* @fm_mutex: serializes ubi_update_fastmap()
* @fm_sem: allows ubi_update_fastmap() to block EBA table changes
* @fm_work: fastmap work queue
- * @attached_by_scanning: this UBI device was attached by the old scanning
- * methold. All fastmap volumes have to be deleted.
*
* @used: RB-tree of used physical eraseblocks
* @erroneous: RB-tree of erroneous used physical eraseblocks
@@ -494,7 +492,6 @@ struct ubi_device {
struct rw_semaphore fm_sem;
struct mutex fm_mutex;
struct work_struct fm_work;
- int attached_by_scanning;
/* Wear-leveling sub-system's stuff */
struct rb_root used;
--
1.7.6.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 9 ---------
drivers/mtd/ubi/fastmap.c | 4 ++++
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index e88521b..1b62d54 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1384,15 +1384,6 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
if (err)
goto out_ai;
- /* TODO: Hmm why this code is not hidden in 'ubi_scan_fastmap()' ? */
- if (ubi->fm) {
- ubi->fm_pool.max_size = ubi->fm->max_pool_size;
- ubi->fm_wl_pool.max_size = ubi->fm->max_wl_pool_size;
-
- ubi_msg("fastmap pool size: %d", ubi->fm_pool.max_size);
- ubi_msg("fastmap WL pool size: %d", ubi->fm_wl_pool.max_size);
- }
-
err = ubi_wl_init(ubi, ai);
if (err)
goto out_vtbl;
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 2c7c350..9b4fd34 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -1096,6 +1096,10 @@ int ubi_scan_fastmap(struct ubi_device *ubi, struct ubi_attach_info *ai)
}
ubi->fm = fm;
+ ubi->fm_pool.max_size = ubi->fm->max_pool_size;
+ ubi->fm_wl_pool.max_size = ubi->fm->max_wl_pool_size;
+ ubi_msg("fastmap pool size: %d", ubi->fm_pool.max_size);
+ ubi_msg("fastmap WL pool size: %d", ubi->fm_wl_pool.max_size);
free_hdr:
ubi_free_vid_hdr(ubi, vh);
--
1.7.6.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 3d9be42..bd3a8f5 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1325,12 +1325,12 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
scan_ai = alloc_ai();
if (!scan_ai)
- goto out_ai;
+ goto out_wl;
err = scan_all(ubi, scan_ai);
if (err) {
kfree(scan_ai);
- goto out_ai;
+ goto out_wl;
}
self_check_eba(ubi, ai, scan_ai);
--
1.7.6.5
Signed-off-by: Richard Weinberger <[email protected]>
---
drivers/mtd/ubi/attach.c | 186 ++++++++++++++++++++++------------------------
1 files changed, 89 insertions(+), 97 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index f1dfb63..e88521b 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -89,15 +89,7 @@
#include <linux/random.h>
#include "ubi.h"
-/*
- * TODO: please, no forward declarations. We do not use them in UBI code.
- * Actually initially I did use them a lot, but when upstreaming, I was asked
- * to remove. Please, follow this convention as well. Please, change globally.
- * I mean, I am already used to that _all_ the code is upside-down, let's keep
- * it that way, or re-structure all the code. :-)
- */
static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai);
-static void destroy_ai(struct ubi_device *ubi, struct ubi_attach_info *ai);
/* Temporary variables used during scanning */
static struct ubi_ec_hdr *ech;
@@ -1132,6 +1124,95 @@ static int late_analysis(struct ubi_device *ubi, struct ubi_attach_info *ai)
}
/**
+ * destroy_av - free volume attaching information.
+ * @av: volume attaching information
+ * @ai: attaching information
+ *
+ * This function destroys the volume attaching information.
+ */
+static void destroy_av(struct ubi_attach_info *ai, struct ubi_ainf_volume *av)
+{
+ struct ubi_ainf_peb *aeb;
+ struct rb_node *this = av->root.rb_node;
+
+ while (this) {
+ if (this->rb_left)
+ this = this->rb_left;
+ else if (this->rb_right)
+ this = this->rb_right;
+ else {
+ aeb = rb_entry(this, struct ubi_ainf_peb, u.rb);
+ this = rb_parent(this);
+ if (this) {
+ if (this->rb_left == &aeb->u.rb)
+ this->rb_left = NULL;
+ else
+ this->rb_right = NULL;
+ }
+
+ kmem_cache_free(ai->aeb_slab_cache, aeb);
+ }
+ }
+ kfree(av);
+}
+
+/**
+ * destroy_ai - destroy attaching information.
+ * @ubi: UBI device object
+ * @ai: attaching information
+ */
+static void destroy_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)
+{
+ struct ubi_ainf_peb *aeb, *aeb_tmp;
+ struct ubi_ainf_volume *av;
+ struct rb_node *rb;
+
+ list_for_each_entry_safe(aeb, aeb_tmp, &ai->alien, u.list) {
+ list_del(&aeb->u.list);
+ kmem_cache_free(ai->aeb_slab_cache, aeb);
+ }
+ list_for_each_entry_safe(aeb, aeb_tmp, &ai->erase, u.list) {
+ list_del(&aeb->u.list);
+ kmem_cache_free(ai->aeb_slab_cache, aeb);
+ }
+ list_for_each_entry_safe(aeb, aeb_tmp, &ai->corr, u.list) {
+ list_del(&aeb->u.list);
+ kmem_cache_free(ai->aeb_slab_cache, aeb);
+ }
+ list_for_each_entry_safe(aeb, aeb_tmp, &ai->free, u.list) {
+ list_del(&aeb->u.list);
+ kmem_cache_free(ai->aeb_slab_cache, aeb);
+ }
+
+ /* Destroy the volume RB-tree */
+ rb = ai->volumes.rb_node;
+ while (rb) {
+ if (rb->rb_left)
+ rb = rb->rb_left;
+ else if (rb->rb_right)
+ rb = rb->rb_right;
+ else {
+ av = rb_entry(rb, struct ubi_ainf_volume, rb);
+
+ rb = rb_parent(rb);
+ if (rb) {
+ if (rb->rb_left == &av->rb)
+ rb->rb_left = NULL;
+ else
+ rb->rb_right = NULL;
+ }
+
+ destroy_av(ai, av);
+ }
+ }
+
+ if (ai->aeb_slab_cache)
+ kmem_cache_destroy(ai->aeb_slab_cache);
+
+ kfree(ai);
+}
+
+/**
* scan_all - scan entire MTD device.
* @ubi: UBI device description object
* @ai: attach info object
@@ -1354,95 +1435,6 @@ out_ai:
}
/**
- * destroy_av - free volume attaching information.
- * @av: volume attaching information
- * @ai: attaching information
- *
- * This function destroys the volume attaching information.
- */
-static void destroy_av(struct ubi_attach_info *ai, struct ubi_ainf_volume *av)
-{
- struct ubi_ainf_peb *aeb;
- struct rb_node *this = av->root.rb_node;
-
- while (this) {
- if (this->rb_left)
- this = this->rb_left;
- else if (this->rb_right)
- this = this->rb_right;
- else {
- aeb = rb_entry(this, struct ubi_ainf_peb, u.rb);
- this = rb_parent(this);
- if (this) {
- if (this->rb_left == &aeb->u.rb)
- this->rb_left = NULL;
- else
- this->rb_right = NULL;
- }
-
- kmem_cache_free(ai->aeb_slab_cache, aeb);
- }
- }
- kfree(av);
-}
-
-/**
- * destroy_ai - destroy attaching information.
- * @ubi: UBI device object
- * @ai: attaching information
- */
-static void destroy_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)
-{
- struct ubi_ainf_peb *aeb, *aeb_tmp;
- struct ubi_ainf_volume *av;
- struct rb_node *rb;
-
- list_for_each_entry_safe(aeb, aeb_tmp, &ai->alien, u.list) {
- list_del(&aeb->u.list);
- kmem_cache_free(ai->aeb_slab_cache, aeb);
- }
- list_for_each_entry_safe(aeb, aeb_tmp, &ai->erase, u.list) {
- list_del(&aeb->u.list);
- kmem_cache_free(ai->aeb_slab_cache, aeb);
- }
- list_for_each_entry_safe(aeb, aeb_tmp, &ai->corr, u.list) {
- list_del(&aeb->u.list);
- kmem_cache_free(ai->aeb_slab_cache, aeb);
- }
- list_for_each_entry_safe(aeb, aeb_tmp, &ai->free, u.list) {
- list_del(&aeb->u.list);
- kmem_cache_free(ai->aeb_slab_cache, aeb);
- }
-
- /* Destroy the volume RB-tree */
- rb = ai->volumes.rb_node;
- while (rb) {
- if (rb->rb_left)
- rb = rb->rb_left;
- else if (rb->rb_right)
- rb = rb->rb_right;
- else {
- av = rb_entry(rb, struct ubi_ainf_volume, rb);
-
- rb = rb_parent(rb);
- if (rb) {
- if (rb->rb_left == &av->rb)
- rb->rb_left = NULL;
- else
- rb->rb_right = NULL;
- }
-
- destroy_av(ai, av);
- }
- }
-
- if (ai->aeb_slab_cache)
- kmem_cache_destroy(ai->aeb_slab_cache);
-
- kfree(ai);
-}
-
-/**
* self_check_ai - check the attaching information.
* @ubi: UBI device description object
* @ai: attaching information
--
1.7.6.5
On Fri, 2012-06-29 at 17:14 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
>
> Fastmap is now disabled by default.
> If you attach an image it will not automatically convert it
> to the fastmap format.
> UBI has a new module parameter "fm_auto".
> If set to 1 UBI will create a fastmap automatically on your
> flash device.
> Please see commit "Add a module parameter to enable fastmap"
> for more details.
One thing I've noticed is that you use vmalloc for some of the buffers
you do I/O on (fm_raw). This is a problem for many ARM platforms because
they cannot do DMA on such buffers. The drivers have to implement
various workarouns for this: either fall back to memcopies or do an
extra copy to a DMA-able buffers.
UBI and UBIFS already to I/O on vmalloc'ed buffers, an I am planning to
fix this, which is not easy. Would be great to not add more of these.
--
Best Regards,
Artem Bityutskiy
Am 30.06.2012 12:43, schrieb Artem Bityutskiy:
> On Fri, 2012-06-29 at 17:14 +0200, Richard Weinberger wrote:
>> This is the next round of UBI fastmap updates.
>>
>> Fastmap is now disabled by default.
>> If you attach an image it will not automatically convert it
>> to the fastmap format.
>> UBI has a new module parameter "fm_auto".
>> If set to 1 UBI will create a fastmap automatically on your
>> flash device.
>> Please see commit "Add a module parameter to enable fastmap"
>> for more details.
>
> One thing I've noticed is that you use vmalloc for some of the buffers
> you do I/O on (fm_raw). This is a problem for many ARM platforms because
> they cannot do DMA on such buffers. The drivers have to implement
> various workarouns for this: either fall back to memcopies or do an
> extra copy to a DMA-able buffers.
>
> UBI and UBIFS already to I/O on vmalloc'ed buffers, an I am planning to
> fix this, which is not easy. Would be great to not add more of these.
There are only two buffers which use vmalloc() both are used to hold the raw fastmap and have
the same size.
If it helps I could preallocate a buffer in ubi-> and use it in both functions ubi_write_fastmap()
and ubi_scan_fastmap().
So basically you want me to get rid of any vmalloc()'ed buffer?
That's not easy.
Thanks,
//richard
On Sat, 2012-06-30 at 12:53 +0200, Richard Weinberger wrote:
> So basically you want me to get rid of any vmalloc()'ed buffer?
> That's not easy.
How big is the buffer. What is its maximum possible size?
--
Best Regards,
Artem Bityutskiy
On Sat, 2012-06-30 at 12:53 +0200, Richard Weinberger wrote:
> So basically you want me to get rid of any vmalloc()'ed buffer?
> That's not easy.
Yeah, I guess it is difficult, we probably can let it be vmalloc() since
we already have plenty...
--
Best Regards,
Artem Bityutskiy
On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
> It fixes all issues pointed out by Shmulik. :-)
I see the following errors when rung UBI tests on nandsim:
[ 3698.041511] UBI error: __wl_get_peb: no free eraseblocks
[ 3698.041781] UBI error: ubi_wl_get_fm_peb: no free eraseblocks
[ 3714.773064] UBI error: __wl_get_peb: no free eraseblocks
[ 3714.773336] UBI error: ubi_wl_get_fm_peb: no free eraseblocks
How can this happen? I do not have any bad blocks.
If I understand correctly, it can be only because of a bug. If I am
correct, could you please add a 'dump_stack()' to improve the error
report?
--
Best Regards,
Artem Bityutskiy
Artem,
Am Thu, 02 Aug 2012 17:12:27 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> > This is the next round of UBI fastmap updates.
> > It fixes all issues pointed out by Shmulik. :-)
>
> I see the following errors when rung UBI tests on nandsim:
>
> [ 3698.041511] UBI error: __wl_get_peb: no free eraseblocks
> [ 3698.041781] UBI error: ubi_wl_get_fm_peb: no free eraseblocks
> [ 3714.773064] UBI error: __wl_get_peb: no free eraseblocks
> [ 3714.773336] UBI error: ubi_wl_get_fm_peb: no free eraseblocks
>
> How can this happen? I do not have any bad blocks.
>
> If I understand correctly, it can be only because of a bug. If I am
> correct, could you please add a 'dump_stack()' to improve the error
> report?
>
This can happen if all PEBs are used and fastmap is unable to find (or
produce) an empty one.
In this case fastmap takes care that no invalid or outdated fastmap is
on flash.
Thanks,
//richard
On Thu, 2012-08-02 at 16:15 +0200, Richard Weinberger wrote:
> > If I understand correctly, it can be only because of a bug. If I am
> > correct, could you please add a 'dump_stack()' to improve the error
> > report?
> >
>
> This can happen if all PEBs are used and fastmap is unable to find (or
> produce) an empty one.
In which situations is this possible? Could you please give an example?
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 17:29:01 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Thu, 2012-08-02 at 16:15 +0200, Richard Weinberger wrote:
> > > If I understand correctly, it can be only because of a bug. If I
> > > am correct, could you please add a 'dump_stack()' to improve the
> > > error report?
> > >
> >
> > This can happen if all PEBs are used and fastmap is unable to find
> > (or produce) an empty one.
>
> In which situations is this possible? Could you please give an
> example?
>
Every time fastmap writes a new fastmap to the flash it tries to get a
new PEB and returns the old one (used for the old fastmap) back to the
WL sub-system.
If no free PEBs are available (E.g Volume is full or the erase worker
is too slow) ubi_wl_get_fm_peb() returns NULL and fastmap reuses the
currently used PEB.
In this situation ubi_wl_get_fm_peb() may trigger such an error message.
If think we should get rid of the message as this is not an error
condition. It's a well known execution path.
The only bad thing that happens in such a situation is that a PEB gets
reused.
BTW: Which version of fastmap are you testing?
Thanks,
//richard
On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
> It fixes all issues pointed out by Shmulik. :-)
Hi Richard,
when I try to attach mtdram (NOR flash), UBI fails:
[ 7106.353791] UBI assert failed in ubi_io_is_bad at 623 (pid 5411)
[ 7106.354253] Pid: 5411, comm: modprobe Not tainted 3.5.0+ #2
[ 7106.354255] Call Trace:
[ 7106.354264] [<ffffffffa003c4c1>] ubi_io_is_bad+0xd1/0x100 [ubi]
[ 7106.354271] [<ffffffffa0042bd9>] scan_peb+0x49/0x740 [ubi]
[ 7106.354287] [<ffffffff8117e6e4>] ? __kmalloc+0x194/0x1e0
[ 7106.354293] [<ffffffffa0047b7c>] ? ubi_debugging_init_dev+0x2c/0x60 [ubi]
[ 7106.354300] [<ffffffffa0044276>] ? ubi_attach+0x66/0x380 [ubi]
[ 7106.354304] [<ffffffffa00442f5>] ubi_attach+0xe5/0x380 [ubi]
[ 7106.354310] [<ffffffffa0035f91>] ubi_attach_mtd_dev+0xa81/0x10e0 [ubi]
[ 7106.354316] [<ffffffffa005c3e8>] ubi_init+0x22d/0xe45 [ubi]
[ 7106.354321] [<ffffffffa005c1bb>] ? ubi_mtd_param_parse+0x1bb/0x1bb [ubi]
[ 7106.354328] [<ffffffff8100203f>] do_one_initcall+0x3f/0x170
[ 7106.354335] [<ffffffff810c69c6>] sys_init_module+0xa16/0x1d40
[ 7106.354345] [<ffffffff8132c5a0>] ? ddebug_add_module+0x100/0x100
[ 7106.354351] [<ffffffff815ffca9>] system_call_fastpath+0x16/0x1b
[ 7106.354353] UBI assert failed in ubi_io_read_ec_hdr at 749 (pid 5411)
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 17:58:50 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> > This is the next round of UBI fastmap updates.
> > It fixes all issues pointed out by Shmulik. :-)
>
> Hi Richard,
>
> when I try to attach mtdram (NOR flash), UBI fails:
>
Hmm, and without fastmap it works fine?
I don't see much fastmap related here.
Anyway, I'll try to reproduce.
Thanks,
//richard
On Thu, 2012-08-02 at 16:59 +0200, Richard Weinberger wrote:
> Am Thu, 02 Aug 2012 17:58:50 +0300
> schrieb Artem Bityutskiy <[email protected]>:
>
> > On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> > > This is the next round of UBI fastmap updates.
> > > It fixes all issues pointed out by Shmulik. :-)
> >
> > Hi Richard,
> >
> > when I try to attach mtdram (NOR flash), UBI fails:
> >
>
> Hmm, and without fastmap it works fine?
Yes.
> I don't see much fastmap related here.
It is related to your changes in attach.c.
> Anyway, I'll try to reproduce.
Please, make sure that UBI tests work fine on mtdram as well.
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 18:18:48 +0300
schrieb Artem Bityutskiy <[email protected]>:
> > Hmm, and without fastmap it works fine?
>
> Yes.
>
> > I don't see much fastmap related here.
>
> It is related to your changes in attach.c.
Okay, I'll dig into the issue.
Thanks,
//richard
On Thu, 2012-08-02 at 16:51 +0200, Richard Weinberger wrote:
> Every time fastmap writes a new fastmap to the flash it tries to get a
> new PEB and returns the old one (used for the old fastmap) back to the
> WL sub-system.
OK.
> If no free PEBs are available (E.g Volume is full or the erase worker
> is too slow) ubi_wl_get_fm_peb() returns NULL and fastmap reuses the
> currently used PEB.
This should not happen. Fastmap should _reserve_ enough of PEBs for it
to operate. It should always find the PEB to write.
Just like if you create a volume maximum possible size, we guarantee
that you can fill it with data, and UBI will find enough PEBs for that.
Just like we always have enough PEBs for the volume table.
The above things are UBI's liabilities.
In the situation when a lot of PEBs became bad, UBI will switch to R/O
mode with a scary message if it notices that it does not have enough
PEBs to satisfy all the liabilities.
And this is why we reserve 2% of PEBs for bad PEBs handling.
> In this situation ubi_wl_get_fm_peb() may trigger such an error message.
> If think we should get rid of the message as this is not an error
> condition. It's a well known execution path.
Unless I am confused, this should lead to switching to R/O mode instead,
just like we do when we write to an LEB and do not find a PEB to map to.
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 19:17:47 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Thu, 2012-08-02 at 16:51 +0200, Richard Weinberger wrote:
> > Every time fastmap writes a new fastmap to the flash it tries to
> > get a new PEB and returns the old one (used for the old fastmap)
> > back to the WL sub-system.
>
> OK.
>
> > If no free PEBs are available (E.g Volume is full or the erase
> > worker is too slow) ubi_wl_get_fm_peb() returns NULL and fastmap
> > reuses the currently used PEB.
>
> This should not happen. Fastmap should _reserve_ enough of PEBs for it
> to operate. It should always find the PEB to write.
What is the benefit?
IOW what is wrong with the current approach?
> Just like if you create a volume maximum possible size, we guarantee
> that you can fill it with data, and UBI will find enough PEBs for
> that.
>
> Just like we always have enough PEBs for the volume table.
>
> The above things are UBI's liabilities.
>
> In the situation when a lot of PEBs became bad, UBI will switch to R/O
> mode with a scary message if it notices that it does not have enough
> PEBs to satisfy all the liabilities.
>
> And this is why we reserve 2% of PEBs for bad PEBs handling.
Of course fastmap could also do something like that, but I don't really
see a benefit in this.
> > In this situation ubi_wl_get_fm_peb() may trigger such an error
> > message. If think we should get rid of the message as this is not
> > an error condition. It's a well known execution path.
>
> Unless I am confused, this should lead to switching to R/O mode
> instead, just like we do when we write to an LEB and do not find a
> PEB to map to.
Why?
If everything goes wrong, fastmap makes sure that no fastmap is on
flash.
In case of a powercut we fall back to scanning mode.
R/O mode is overkill IMHO.
Thanks,
//richard
Richard,
On Thu, 2012-08-02 at 18:32 +0200, Richard Weinberger wrote:
> > This should not happen. Fastmap should _reserve_ enough of PEBs for it
> > to operate. It should always find the PEB to write.
>
> What is the benefit?
> IOW what is wrong with the current approach?
Several reasons. The main is: fastmap will start consuming PEBs reserved
for volumes when the amount of available PEBs is just enough to map all
LEBs. This will break UBI liability.
> Why?
> If everything goes wrong, fastmap makes sure that no fastmap is on
> flash.
> In case of a powercut we fall back to scanning mode.
> R/O mode is overkill IMHO.
So can I interpret this the following way. Not only fastmap give no
guarantees that it exists after an unclean reboot, it does not even give
guarantees that it exists after a clean reboot.
Unless I am confused, the fastmap design is over-simplified.
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 19:45:30 +0300
schrieb Artem Bityutskiy <[email protected]>:
> Richard,
>
> On Thu, 2012-08-02 at 18:32 +0200, Richard Weinberger wrote:
> > > This should not happen. Fastmap should _reserve_ enough of PEBs
> > > for it to operate. It should always find the PEB to write.
> >
> > What is the benefit?
> > IOW what is wrong with the current approach?
>
> Several reasons. The main is: fastmap will start consuming PEBs
> reserved for volumes when the amount of available PEBs is just enough
> to map all LEBs. This will break UBI liability.
Fastmap is also just a volume.
But if you want I can reserve PEBs for it.
> > Why?
> > If everything goes wrong, fastmap makes sure that no fastmap is on
> > flash.
> > In case of a powercut we fall back to scanning mode.
> > R/O mode is overkill IMHO.
>
> So can I interpret this the following way. Not only fastmap give no
> guarantees that it exists after an unclean reboot, it does not even
> give guarantees that it exists after a clean reboot.
As I said several times before, fastmap was designed to be able to
fall back to scanning mode.
And yes, if there is currently no fastmap on flash (because you
attached from an old UBI volume) and there are no free PEBs you'll
have no fastmap on flash.
In all other cases you'll have one. At detach time fastmap checks
whether a fastmap is installed or not and installs one if needed.
> Unless I am confused, the fastmap design is over-simplified.
KISS.
Thanks,
//richard
On 08/02/2012 09:45 AM, Artem Bityutskiy wrote:
> Richard,
>
> On Thu, 2012-08-02 at 18:32 +0200, Richard Weinberger wrote:
>>> This should not happen. Fastmap should _reserve_ enough of PEBs for it
>>> to operate. It should always find the PEB to write.
>>
>> What is the benefit?
>> IOW what is wrong with the current approach?
>
> Several reasons. The main is: fastmap will start consuming PEBs reserved
> for volumes when the amount of available PEBs is just enough to map all
> LEBs. This will break UBI liability.
I'm don't understand what "UBI liability" is. Can you please clarify?
What breaks if the PEBs get consumed?
>
>> Why?
>> If everything goes wrong, fastmap makes sure that no fastmap is on
>> flash.
>> In case of a powercut we fall back to scanning mode.
>> R/O mode is overkill IMHO.
>
> So can I interpret this the following way. Not only fastmap give no
> guarantees that it exists after an unclean reboot, it does not even give
> guarantees that it exists after a clean reboot.
>
> Unless I am confused, the fastmap design is over-simplified.
Fastmap is an optimization. Maybe I'm missing something, but
I'm not sure why, if the optimization stopped working, you
would want to reduce the functionality of the file system.
=============================
Tim Bird
Architecture Group Chair, CE Workgroup of the Linux Foundation
Senior Staff Engineer, Sony Network Entertainment
=============================
Am Thu, 2 Aug 2012 10:03:04 -0700
schrieb Tim Bird <[email protected]>:
> >> If everything goes wrong, fastmap makes sure that no fastmap is on
> >> flash.
> >> In case of a powercut we fall back to scanning mode.
> >> R/O mode is overkill IMHO.
> >
> > So can I interpret this the following way. Not only fastmap give no
> > guarantees that it exists after an unclean reboot, it does not even
> > give guarantees that it exists after a clean reboot.
> >
> > Unless I am confused, the fastmap design is over-simplified.
>
> Fastmap is an optimization. Maybe I'm missing something, but
> I'm not sure why, if the optimization stopped working, you
> would want to reduce the functionality of the file system.
That's *exactly* my point.
If fastmap is available - fine, we have fast boot - yay!
But if something really nasty happens we can safely fall back
to scanning mode.
And fastmap is designed to allow this.
Thanks,
//richard
Hi Tim,
On Thu, 2012-08-02 at 10:03 -0700, Tim Bird wrote:
> I'm don't understand what "UBI liability" is. Can you please clarify?
> What breaks if the PEBs get consumed?
let me try. Let's forget about bad blocks and assume we are talking
about NOR flash. For simplicity.
Let's also first forget about fastmap so far and talk about the current
design.
Suppose you have 100 PEBs on your flash. Suppose UBI reserves 10 for its
internal needs (volume table, etc). 90 PEBs are available to the user.
User now can create one or many volumes, but the overall size of the
volumes cannot be larger than 90 LEBs.
This means that UBI guarantees that you can always fill all volumes with
data there will always be enough PEBs to map to. This is UBI liability.
UBI will not allow you to create a volume of 100 LEBs because in this
case it will not be able to guarantee that all LEBs will be writable.
I have invented this "liability" term on the spot actually. It basically
means what UBI already "promised", what it reserved an put aside.
Now let's add fastmap support to the picture.
Suppose fastmap took another 10 PEBs and now we have 80 PEBs for the
user.
The user can create a volume of 80 LEBs in size. And UBI has to
guarantee that the user can at any point of time fill all of them with
data.
This means that fastmap in no circumstances can grab any more than 10
PEBs, because they are all reserved, UBI liability is 80 PEBs.
On other words, fastmap has to know how much PEBs it needs at the UBI
initialization time, and reserve them. The _maximum_ value.
The same way other UBI sub-systems do. E.g., the volume table code
reserves 2 PEBs, because this is the maximum it needs at any point of
time. The WL subsystem reserves 1 PEB.
Of course this is not about reserving any specific PEB, this is just
accounting - we have a couple of variable for reserved PEBs count.
So let's return to the error messages I spotted. They say that fastmap
needs a PEB but cannot find one. The flash is nandsim and has no
badblocks. Why fastmap did not find one? Because it did not reserve
enough. And UBI tests create volumes of maximum possible size and fill
it with data, so all available PEBs are mapped and thus, used.
What this means that the following situation is possible: the UBI volume
is not fully filled yet and not all LEBs are mapped, so there are
available PEBs, and fastmap successfully grows and reduces the amount of
available PEBs. And when the user writes more data, he gets -ENOSPC.
And this is basically the problem I indicated.
To make my description complete, let's add NAND to the picture. We
simply reserve 2% (by default, it is configurable) of PEBs for bad
blocks handling. This is because vendors typically say that this is the
maximum amount for the flash life-time.
If NAND wears-out a lot, and we run out of reserved PEBs, we switch to
R/O mode, because we cannot anymore keep our "promise", the liability.
You can look at it this way. If you create an UBI volume, and mount it
with UBIFS, you usually expect that all the free file-system space is
available to you.
You probably will be disappointed if you write your file and get -ENOSPC
because fastmap does grew and consumed a PEB which which was promised to
your volume.
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 20:40:00 +0300
schrieb Artem Bityutskiy <[email protected]>:
> Hi Tim,
>
> On Thu, 2012-08-02 at 10:03 -0700, Tim Bird wrote:
> > I'm don't understand what "UBI liability" is. Can you please
> > clarify? What breaks if the PEBs get consumed?
>
> let me try. Let's forget about bad blocks and assume we are talking
> about NOR flash. For simplicity.
>
> Let's also first forget about fastmap so far and talk about the
> current design.
>
> Suppose you have 100 PEBs on your flash. Suppose UBI reserves 10 for
> its internal needs (volume table, etc). 90 PEBs are available to the
> user.
>
> User now can create one or many volumes, but the overall size of the
> volumes cannot be larger than 90 LEBs.
>
> This means that UBI guarantees that you can always fill all volumes
> with data there will always be enough PEBs to map to. This is UBI
> liability.
>
> UBI will not allow you to create a volume of 100 LEBs because in this
> case it will not be able to guarantee that all LEBs will be writable.
>
> I have invented this "liability" term on the spot actually. It
> basically means what UBI already "promised", what it reserved an put
> aside.
>
> Now let's add fastmap support to the picture.
>
> Suppose fastmap took another 10 PEBs and now we have 80 PEBs for the
> user.
>
> The user can create a volume of 80 LEBs in size. And UBI has to
> guarantee that the user can at any point of time fill all of them with
> data.
>
> This means that fastmap in no circumstances can grab any more than 10
> PEBs, because they are all reserved, UBI liability is 80 PEBs.
>
> On other words, fastmap has to know how much PEBs it needs at the UBI
> initialization time, and reserve them. The _maximum_ value.
>
> The same way other UBI sub-systems do. E.g., the volume table code
> reserves 2 PEBs, because this is the maximum it needs at any point of
> time. The WL subsystem reserves 1 PEB.
>
> Of course this is not about reserving any specific PEB, this is just
> accounting - we have a couple of variable for reserved PEBs count.
>
> So let's return to the error messages I spotted. They say that fastmap
> needs a PEB but cannot find one. The flash is nandsim and has no
> badblocks. Why fastmap did not find one? Because it did not reserve
> enough. And UBI tests create volumes of maximum possible size and fill
> it with data, so all available PEBs are mapped and thus, used.
>
> What this means that the following situation is possible: the UBI
> volume is not fully filled yet and not all LEBs are mapped, so there
> are available PEBs, and fastmap successfully grows and reduces the
> amount of available PEBs. And when the user writes more data, he gets
> -ENOSPC.
>
> And this is basically the problem I indicated.
>
> To make my description complete, let's add NAND to the picture. We
> simply reserve 2% (by default, it is configurable) of PEBs for bad
> blocks handling. This is because vendors typically say that this is
> the maximum amount for the flash life-time.
>
> If NAND wears-out a lot, and we run out of reserved PEBs, we switch to
> R/O mode, because we cannot anymore keep our "promise", the liability.
>
> You can look at it this way. If you create an UBI volume, and mount it
> with UBIFS, you usually expect that all the free file-system space is
> available to you.
>
> You probably will be disappointed if you write your file and get
> -ENOSPC because fastmap does grew and consumed a PEB which which was
> promised to your volume.
>
Okay, then let's explicitly reserve a few PEBs for fastmap.
This should be very easy task.
How much PEB should be reserved? 2 x sizeof(fastmap)?
Thanks,
//richard
On Thu, 2012-08-02 at 10:03 -0700, Tim Bird wrote:
> > So can I interpret this the following way. Not only fastmap give no
> > guarantees that it exists after an unclean reboot, it does not even give
> > guarantees that it exists after a clean reboot.
> >
> > Unless I am confused, the fastmap design is over-simplified.
>
> Fastmap is an optimization. Maybe I'm missing something, but
> I'm not sure why, if the optimization stopped working, you
> would want to reduce the functionality of the file system.
Fastmap gives huge improvement in attach time. So big that it becomes
not just optimization, but a selling feature. Or very important
optimization.
If you design a system which requires 1s startup time, you probably want
to always guarantee 1s startup time. E.g., if we are talking about a car
system.
You probably may get away with the fact that in case of power cut your
system starts up 10 seconds at the first boot (no fastmap).
But you probably would be disappointed if I say that even if you do
_not_ have power cuts, your system may still startup 10 seconds,
although most of the times it will take 1s.
Does this description help to accept my POW that while we cannot
simplify fastmap and give no improvement for the power cut cases, it is
quite important to guarantee that in normal cases fastmap is always
there, and UBI will always be fast.
If I buy a car which runs 200Km/h on the asphalt, I am OK if it cannot
do this on the cross-country trails, but I am not OK if it sometimes
cannot do 200Km/h even on the asphalt, when the moon is blue.
--
Best Regards,
Artem Bityutskiy
On Thu, 2012-08-02 at 19:45 +0200, Richard Weinberger wrote:
> This should be very easy task.
Right. But unfortunately, I had to spend a lot of time writing lengthy
e-mails. You could hold your horses for a minute, ask specific questions
and find out what was my concern.
> How much PEB should be reserved? 2 x sizeof(fastmap)?
Is there any reason why it cannot be the _exact_ maximum number? Not
more and not less.
If I understand correctly, fastmap size is a function of total PEBs
count. You should be able to calculate the maximum size precisely.
--
Best Regards,
Artem Bityutskiy
Am Thu, 02 Aug 2012 20:59:28 +0300
schrieb Artem Bityutskiy <[email protected]>:
> > How much PEB should be reserved? 2 x sizeof(fastmap)?
>
> Is there any reason why it cannot be the _exact_ maximum number? Not
> more and not less.
The fastmap size is an exact number.
> If I understand correctly, fastmap size is a function of total PEBs
> count. You should be able to calculate the maximum size precisely.
It does.
I was thinking of 2 x sizeof(fastmap) to have reserved PEBs for the
currently used fastmap and PEBs for the new to be installed fastmap.
Thanks,
//richard
On Thu, 2012-08-02 at 20:03 +0200, Richard Weinberger wrote:
> > If I understand correctly, fastmap size is a function of total PEBs
> > count. You should be able to calculate the maximum size precisely.
>
> It does.
> I was thinking of 2 x sizeof(fastmap) to have reserved PEBs for the
> currently used fastmap and PEBs for the new to be installed fastmap.
Up to you. If you are fine with the overhead, you can go for 2x, I do
not have objections. But would be nice to include the overhead numbers
when you submit the patches. Also print on initialization.
--
Best Regards,
Artem Bityutskiy
On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
> It fixes all issues pointed out by Shmulik. :-)
>
> If you want to test fastmap you can use my git repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v17
Richard, would you please rename the 'fm_auto' kernel parameter to
"fastmap_auto", which is a bit more user-friendly.
Thanks!
--
Best Regards,
Artem Bityutskiy
On Thu, 2012-08-02 at 21:50 +0300, Artem Bityutskiy wrote:
> On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> > This is the next round of UBI fastmap updates.
> > It fixes all issues pointed out by Shmulik. :-)
> >
> > If you want to test fastmap you can use my git repo:
> > git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v17
>
> Richard, would you please rename the 'fm_auto' kernel parameter to
> "fastmap_auto", which is a bit more user-friendly.
Sorry, I meant module parameter. Too late, time to go home.
--
Best Regards,
Artem Bityutskiy
On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
> It fixes all issues pointed out by Shmulik. :-)
>
> If you want to test fastmap you can use my git repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v17
Richard,
I've added 'stress-test.sh' script to the UBI tests. This script runs
UBI tests on nandsim of different geometry. I plan to extend it further:
add mtdram tests, test with bit-flips emulation enabled, may be
something else.
We need to make sure all the tests pass and fastmap does not introduce
regressions.
Feel free to send patches. I am going to extend the test today, so 'git
pull' from time to time.
The tests will run very long time, so for debugging you can always
comment out unneeded things.
ATM, I have only nandsim tests with different geometry: 64MiB to 1GiB
total size, 2KiB and 512 byte pages, 16-256KiB eraseblocks.
Currently I am running this to unpatched UBI to check if they really
pass.
I tried to run it on the patched UBI and hit this issue:
======================================================================
16MiB nandsim with 16KiB PEB, 512KiB NAND pages, fastmap enabled
Loaded NAND simulator (16MiB, 16KiB eraseblock, 512 bytes NAND page)
Running mkvol_basic /dev/ubi0
Running mkvol_bad /dev/ubi0
Running mkvol_paral /dev/ubi0
Running rsvol /dev/ubi0
Running io_basic /dev/ubi0
[io_basic] test_basic():70: function write() failed with error 28 (No
space left on device)
[io_basic] test_basic():70: written = 15808000, ret = -1
Error: io_basic failed
FAILURE
======================================================================
On non-patched UBI it works. I think it is exactly what I was talking
about yesterday - fastmap grows unexpectedly because you do not reserve
the space.
Enjoy :-)
--
Best Regards,
Artem Bityutskiy
Am Fri, 03 Aug 2012 11:47:17 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> > This is the next round of UBI fastmap updates.
> > It fixes all issues pointed out by Shmulik. :-)
> >
> > If you want to test fastmap you can use my git repo:
> > git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v17
>
> Richard,
>
> I've added 'stress-test.sh' script to the UBI tests. This script runs
> UBI tests on nandsim of different geometry. I plan to extend it
> further: add mtdram tests, test with bit-flips emulation enabled, may
> be something else.
>
> We need to make sure all the tests pass and fastmap does not introduce
> regressions.
>
> Feel free to send patches. I am going to extend the test today, so
> 'git pull' from time to time.
>
> The tests will run very long time, so for debugging you can always
> comment out unneeded things.
>
> ATM, I have only nandsim tests with different geometry: 64MiB to 1GiB
> total size, 2KiB and 512 byte pages, 16-256KiB eraseblocks.
>
> Currently I am running this to unpatched UBI to check if they really
> pass.
>
> I tried to run it on the patched UBI and hit this issue:
>
> ======================================================================
> 16MiB nandsim with 16KiB PEB, 512KiB NAND pages, fastmap enabled
> Loaded NAND simulator (16MiB, 16KiB eraseblock, 512 bytes NAND page)
> Running mkvol_basic /dev/ubi0
> Running mkvol_bad /dev/ubi0
> Running mkvol_paral /dev/ubi0
> Running rsvol /dev/ubi0
> Running io_basic /dev/ubi0
> [io_basic] test_basic():70: function write() failed with error 28 (No
> space left on device)
> [io_basic] test_basic():70: written = 15808000, ret = -1
> Error: io_basic failed
> FAILURE
> ======================================================================
>
> On non-patched UBI it works. I think it is exactly what I was talking
> about yesterday - fastmap grows unexpectedly because you do not
> reserve the space.
Yeah, this must be the case.
I'm wondering why the test passes with the default nandsim settings?
I have also tested with other flash sizes and did lots of tests on
real hardware.
Thanks,
//richard
Hi,
On Thu, 2 Aug 2012 19:45:38 +0200 Richard Weinberger <[email protected]> wrote:
> Okay, then let's explicitly reserve a few PEBs for fastmap.
> This should be very easy task.
Need to consider what's expected when migrating from a former non-FM
UBI system to an FM enabled system, in the case where all PEBs where
consumed (reserved) in the former system.
> How much PEB should be reserved? 2 x sizeof(fastmap)?
Since FM does not use EBA's atomic LEB change when writing the new
fastmap, but instead implements its own FM "leb change" internally -
then reserving 2x is needed if we'd like to avoid reusing the same
fastmap PEB.
Regards,
Shmulik
Am 05.08.2012 10:23, schrieb Shmulik Ladkani:
> On Thu, 2 Aug 2012 19:45:38 +0200 Richard Weinberger <[email protected]> wrote:
>> Okay, then let's explicitly reserve a few PEBs for fastmap.
>> This should be very easy task.
>
> Need to consider what's expected when migrating from a former non-FM
> UBI system to an FM enabled system, in the case where all PEBs where
> consumed (reserved) in the former system.
If no PEBs are available no fastmap can be installed.
*Maybe* we can steal some PEBs which reserved for bad block handling.
>> How much PEB should be reserved? 2 x sizeof(fastmap)?
>
> Since FM does not use EBA's atomic LEB change when writing the new
> fastmap, but instead implements its own FM "leb change" internally -
> then reserving 2x is needed if we'd like to avoid reusing the same
> fastmap PEB.
Yep.
Thanks,
//richard
Am 02.08.2012 16:58, schrieb Artem Bityutskiy:
> On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
>> This is the next round of UBI fastmap updates.
>> It fixes all issues pointed out by Shmulik. :-)
>
> Hi Richard,
>
> when I try to attach mtdram (NOR flash), UBI fails:
Fastmap works fine with mtdram and NOR flash.
But if your MTD device has less than UBI_FM_MAX_START (64) PEBs
ubi_io_is_bad() will trigger.
The fix is to disable Fastmap if you have not enough PEBs.
I think we enable fastmap only if a MTD device has more than
UBI_FM_MAX_START*2 PEBs.
Any comments?
Thanks,
//richard
On Mon, 2012-08-06 at 19:36 +0200, Richard Weinberger wrote:
> I think we enable fastmap only if a MTD device has more than
> UBI_FM_MAX_START*2 PEBs.
> Any comments?
With double space one can make it power-cut tolerant, because you should
be able to have either old or new fastmap at any point of time.
--
Best Regards,
Artem Bityutskiy
Am 07.08.2012 06:21, schrieb Artem Bityutskiy:
> On Mon, 2012-08-06 at 19:36 +0200, Richard Weinberger wrote:
>> I think we enable fastmap only if a MTD device has more than
>> UBI_FM_MAX_START*2 PEBs.
>> Any comments?
>
> With double space one can make it power-cut tolerant, because you should
> be able to have either old or new fastmap at any point of time.
UBI_FM_MAX_START*2 has nothing do to with the Fastmap size.
IMHO we need a threshold where Fastmap makes sense.
Technically Fastmap can only be used if a MTD device has >= UBI_FM_MAX_START
PEBs.
But does this makes sense? Fastmap was invented to speedup attaching on *large* MTDs,
The benefit in small MTDs is very little.
Thanks,
//richard
On Tue, 2012-08-07 at 09:29 +0200, Richard Weinberger wrote:
> Am 07.08.2012 06:21, schrieb Artem Bityutskiy:
> > On Mon, 2012-08-06 at 19:36 +0200, Richard Weinberger wrote:
> >> I think we enable fastmap only if a MTD device has more than
> >> UBI_FM_MAX_START*2 PEBs.
> >> Any comments?
> >
> > With double space one can make it power-cut tolerant, because you should
> > be able to have either old or new fastmap at any point of time.
>
> UBI_FM_MAX_START*2 has nothing do to with the Fastmap size.
> IMHO we need a threshold where Fastmap makes sense.
> Technically Fastmap can only be used if a MTD device has >= UBI_FM_MAX_START
> PEBs.
> But does this makes sense? Fastmap was invented to speedup attaching on *large* MTDs,
> The benefit in small MTDs is very little.
You may measure when it starts being reasonable to have fastmap enabled,
or interpolate the data you already have (everything is roughly linear,
should be rather easy).
But of course small flashes do not need fastmap.
Also, did you say in the past that you are going to come up with a
document describing the design, its cons and pros, limitations, and some
numbers. It would help a lot. Even a limited document would be better
than none.
Speaking about numbers, how long does it take to re-write fastmap on a
given flash (say, 1GiB or larger, depending on what HW you have)? How
big is fastmap for a given partitions size? I'd really prefer to see
this information in a document, e.g., a text file, rather than spread
over many e-mails.
Thanks!
--
Best Regards,
Artem Bityutskiy
On Mon, 2012-07-09 at 14:18 +0200, Richard Weinberger wrote:
> This is the next round of UBI fastmap updates.
> It fixes all issues pointed out by Shmulik. :-)
>
> If you want to test fastmap you can use my git repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/rw/ubi2.git ubi2/v17
One thing which just came to my mind why looking at other MTD e-mails. I
do not know if it is relevant for fastmap or not, just want you to
check.
In UBIFS we have we have the "fixup" feature which is used to
work-around dumb flashers which are present in many factories:
http://www.linux-mtd.infradead.org/faq/ubifs.html#L_free_space_fixup
This is because the "correct" UBI flasher should be able to skip empty
space when flashing, see here:
http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
So with this fixup flag UBIFS will basically read all its eraseblocks
which have empty spece which it will use, and write them back. Just to
make the empty space writable again. This is done only once on the very
first boot.
We do not do anything like this in UBI because UBI does not need this,
it does not have any complex data structures on the media.
With fastmap - I am unsure. I think it is not a problem, because
probably you never append more data, you just re-write everything,
right?
--
Best Regards,
Artem Bityutskiy
Am Fri, 17 Aug 2012 16:11:55 +0300
schrieb Artem Bityutskiy <[email protected]>:
> We do not do anything like this in UBI because UBI does not need this,
> it does not have any complex data structures on the media.
>
> With fastmap - I am unsure. I think it is not a problem, because
> probably you never append more data, you just re-write everything,
> right?
Yep, the on-flash fastmap is immutable.
Thanks,
//richard
On Fri, 2012-08-17 at 15:33 +0200, Richard Weinberger wrote:
> Am Fri, 17 Aug 2012 16:11:55 +0300
> schrieb Artem Bityutskiy <[email protected]>:
> > We do not do anything like this in UBI because UBI does not need this,
> > it does not have any complex data structures on the media.
> >
> > With fastmap - I am unsure. I think it is not a problem, because
> > probably you never append more data, you just re-write everything,
> > right?
>
> Yep, the on-flash fastmap is immutable.
Here the question is - if you have PEB containing fastmap data,
half-filled. Will you ever add more data there to the empty space in the
PEB or not?
--
Best Regards,
Artem Bityutskiy
Am Fri, 17 Aug 2012 16:41:24 +0300
schrieb Artem Bityutskiy <[email protected]>:
> On Fri, 2012-08-17 at 15:33 +0200, Richard Weinberger wrote:
> > Am Fri, 17 Aug 2012 16:11:55 +0300
> > schrieb Artem Bityutskiy <[email protected]>:
> > > We do not do anything like this in UBI because UBI does not need
> > > this, it does not have any complex data structures on the media.
> > >
> > > With fastmap - I am unsure. I think it is not a problem, because
> > > probably you never append more data, you just re-write everything,
> > > right?
> >
> > Yep, the on-flash fastmap is immutable.
>
> Here the question is - if you have PEB containing fastmap data,
> half-filled. Will you ever add more data there to the empty space in
> the PEB or not?
>
No, data is never added to a half-filled PEB.
Fastmap does not share PEBs with other sub-systems by design.
Thanks,
//richard
On Fri, 2012-08-17 at 15:43 +0200, Richard Weinberger wrote:
> No, data is never added to a half-filled PEB.
OK.
> Fastmap does not share PEBs with other sub-systems by design.
It could in theory add own data to own PEBs.
--
Best Regards,
Artem Bityutskiy