Programming ubinized images

Programming ubinized images

[Matthew L. Creech]

Hi,

I'm curious about the implications of these notes in the UBI documentation:

http://www.linux-mtd.infradead.org/faq/ubi.html#L_ecc_error
http://www.linux-mtd.infradead.org/faq/ubifs.html#L_why_ubiformat
http://www.linux-mtd.infradead.org/faq/ubifs.html#L_mkfubifs
http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo

which seem to be saying that:

1. A ubinized image can contain empty (all-0xff) pages at the end
2. At run-time, it will assume that these pages are completely erased
(including corresponding OOB areas)
3. Standard NAND flashing utilities will view the 0xff pages as normal
data and program their OOB areas
4. This will cause errors when UBIFS tries to use these areas, because
it double-programs OOB/ECC (presumably resulting in some sort of
ANDing of bits)

Does this mean that an off-the-shelf NAND programmer can't be used to
program UBIFS images onto a flash device?  Or perhaps that I need to
be post-processing the ubinized images to strip out empty pages from
the end of the image before giving it to the production line?

We encountered one case in which we were re-flashing a device for
testing using U-Boot's "nand erase", and got the "ubi_io_read: error
-74" error from the FAQ.  That's no big deal, since we never do this
in the field, and clearly "nand erase" isn't something we'd want to do
even without this problem since it loses erase-counter info.

However, I'm wondering if some of the persistent UBIFS problems we've
been experiencing since last year:

http://lists.infradead.org/pipermail/linux-mtd/2010-July/031069.html

could be occurring simply because we're not stripping empty pages from
the end of the ubinized image before programming.  Does this seem like
a possibility, or do I need to keep looking elsewhere?  Any tips are
appreciated, thanks!

-- 
Matthew L. Creech

Re: Programming ubinized images

[Ricard Wanderlof]

On Mon, 25 Apr 2011, Matthew L. Creech wrote:

> Hi,
>
> I'm curious about the implications of these notes in the UBI documentation:
>
> http://www.linux-mtd.infradead.org/faq/ubi.html#L_ecc_error
> http://www.linux-mtd.infradead.org/faq/ubifs.html#L_why_ubiformat
> http://www.linux-mtd.infradead.org/faq/ubifs.html#L_mkfubifs
> http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
>
> which seem to be saying that:
>
> 1. A ubinized image can contain empty (all-0xff) pages at the end
> 2. At run-time, it will assume that these pages are completely erased
> (including corresponding OOB areas)
> 3. Standard NAND flashing utilities will view the 0xff pages as normal
> data and program their OOB areas

Yes,

> 4. This will cause errors when UBIFS tries to use these areas, because
> it double-programs OOB/ECC (presumably resulting in some sort of
> ANDing of bits)

Not quite, the problem is that NAND flash doesn't allow programming the 
same area more than a couple of times (before it is erased again), even if 
it was 'programmed' to all-ones the first time. It's not that the bits get 
ANDed, it's that the bit cells aren't designed to be programmed more than 
two or three times, and mtd/UBI uses all those times for its own use. So 
you end up operating the device out of spec, which turns out to fail for 
certain devices.

> Does this mean that an off-the-shelf NAND programmer can't be used to
> program UBIFS images onto a flash device?

That is my understanding, allthough 'off-the-shelf' is a pretty wide 
generalization, it would be conceivable that some programmers can handle 
this, although I have no idea if this in fact is the case.

> Or perhaps that I need to be post-processing the ubinized images to 
> strip out empty pages from the end of the image before giving it to the 
> production line?

That should work too.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

Re: Programming ubinized images

[Artem Bityutskiy]

On Mon, 2011-04-25 at 14:37 -0400, Matthew L. Creech wrote:
> Hi,
> 
> I'm curious about the implications of these notes in the UBI documentation:
> 
> http://www.linux-mtd.infradead.org/faq/ubi.html#L_ecc_error
> http://www.linux-mtd.infradead.org/faq/ubifs.html#L_why_ubiformat
> http://www.linux-mtd.infradead.org/faq/ubifs.html#L_mkfubifs
> http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo
> 
> which seem to be saying that:
> 
> 1. A ubinized image can contain empty (all-0xff) pages at the end

Note, at the end of eraseblocks, not just at the endo of the image. This
means that in the middle of the image you may have an eraseblock with
empty pages at the end of this eraseblock.

> 2. At run-time, it will assume that these pages are completely erased
> (including corresponding OOB areas)

Right. Only UBIFS does this, not UBI, though.

> 3. Standard NAND flashing utilities will view the 0xff pages as normal
> data and program their OOB areas

Yes :-(

> 4. This will cause errors when UBIFS tries to use these areas, because
> it double-programs OOB/ECC (presumably resulting in some sort of
> ANDing of bits)

Well, not on all NANDs, but yes, this is right.

> Does this mean that an off-the-shelf NAND programmer can't be used to
> program UBIFS images onto a flash device?

Yes, if your flash cannot survive programming with all 0xFFs and then
with real data. OneNAND we used in N900 could easily survive this
because ECC for all 0xFFs was 0xFF. But I guess this is more of a
problem nowadays.

>   Or perhaps that I need to
> be post-processing the ubinized images to strip out empty pages from
> the end of the image before giving it to the production line?

No, because this is not only about the end of the image, but end of
eraseblocks.

> We encountered one case in which we were re-flashing a device for
> testing using U-Boot's "nand erase", and got the "ubi_io_read: error
> -74" error from the FAQ.  That's no big deal, since we never do this
> in the field, and clearly "nand erase" isn't something we'd want to do
> even without this problem since it loses erase-counter info.

OK.

> However, I'm wondering if some of the persistent UBIFS problems we've
> been experiencing since last year:
> 
> http://lists.infradead.org/pipermail/linux-mtd/2010-July/031069.html
> 
> could be occurring simply because we're not stripping empty pages from
> the end of the ubinized image before programming.  Does this seem like
> a possibility, or do I need to keep looking elsewhere?  Any tips are
> appreciated, thanks!

Sure, it might easily be! But if this is the case for you you should be
able to catch this by enabling UBI debugging extra checks. How
up-to-date your UBI is, but relatively recently I added an extra check
that will read all the written data back and check that it is correct.
If you have a "bad" 0xFF page, then you write there, UBI will read it
back just after write and compare with the data it wrote. If the data
does not match, it'll complain. So you can stress-test your setup with
UBI extra checks enabled. This is done by the 'ubi_dbg_check_write()'
function.

We could teach the UBIFS tools and the kernel to deal with these things.
It is possible to do with a special flag in the UBIFS superblock which
would say - this FS has been just flashed, do not use space in
half-filled eraseblocks! Then probably we could go through these
half-filled eraseblocks and fix them up, and then remove that flag.


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: Programming ubinized images

[Matthew L. Creech]

On Tue, Apr 26, 2011 at 4:00 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>>   Or perhaps that I need to
>> be post-processing the ubinized images to strip out empty pages from
>> the end of the image before giving it to the production line?
>
> No, because this is not only about the end of the image, but end of
> eraseblocks.
>

Oh, I see.  So it's probably not feasible to leave out those portions
of the UBI image, unless the NAND programmer/software supports
specifying "holes" in the source image or something like that?

>
> Sure, it might easily be! But if this is the case for you you should be
> able to catch this by enabling UBI debugging extra checks. How
> up-to-date your UBI is, but relatively recently I added an extra check
> that will read all the written data back and check that it is correct.
> If you have a "bad" 0xFF page, then you write there, UBI will read it
> back just after write and compare with the data it wrote. If the data
> does not match, it'll complain. So you can stress-test your setup with
> UBI extra checks enabled. This is done by the 'ubi_dbg_check_write()'
> function.
>

Is 2.6.37 new enough?  It has ubi_dbg_check_write() anyway, right now
it's just #defined away with our config (paranoid checks aren't
enabled).

Unfortunately, the problem happens just often enough to be a serious
pain in the neck, but not often enough for us to catch it with a debug
build (so far).  But maybe it's possible to aggrevate it by repeating
the (bad) flash procedure - "nand erase" followed by flashing the
ubinized image, then doing lots of I/O on the subsequently mounted
UBIFS.

> We could teach the UBIFS tools and the kernel to deal with these things.
> It is possible to do with a special flag in the UBIFS superblock which
> would say - this FS has been just flashed, do not use space in
> half-filled eraseblocks! Then probably we could go through these
> half-filled eraseblocks and fix them up, and then remove that flag.
>

Makes sense.  I might take a stab at this later this week, since it
sounds like something along these lines is required if we plan to use
an industrial (fairly dumb) NAND programmer to flash ubinized UBIFS
images.

Thanks Artem

-- 
Matthew L. Creech

Re: Programming ubinized images

[Artem Bityutskiy]

On Tue, 2011-04-26 at 04:48 -0400, Matthew L. Creech wrote:
> 
> Makes sense.  I might take a stab at this later this week, since it
> sounds like something along these lines is required if we plan to use
> an industrial (fairly dumb) NAND programmer to flash ubinized UBIFS
> images. 

Yeah, we were able to modify the production flasher so it dropped
all-0xFF NAND pages, and we did not foresee that this might be a problem
of others.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: Programming ubinized images

[Matthew L. Creech]

On Tue, Apr 26, 2011 at 4:00 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> We could teach the UBIFS tools and the kernel to deal with these things.
> It is possible to do with a special flag in the UBIFS superblock which
> would say - this FS has been just flashed, do not use space in
> half-filled eraseblocks! Then probably we could go through these
> half-filled eraseblocks and fix them up, and then remove that flag.
>

Hi Artem, could you take a look at this patch when you get a chance?

My method for finding & cleaning the in-use LEBs is hacky: I just loop
through all the LEBs in the FS, look up each one in the LPT, and
"recover" anything not flagged as empty.  I *think* that should result
in LEBs that have trailing empty space (SCANNED_EMPTY_SPACE) being
re-mapped to a new PEB (via fix_unclean_leb()), which would clean up
the empty pages.  So far I haven't gotten that condition to trigger
with a real UBIFS image, so I'm not sure if it works correctly yet.

I may be way off on the wrong track though, so I just wanted to see if
you think this seems like a reasonable way of doing it.  Thanks!


diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index f7515bd..fc2c48b 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -316,6 +316,8 @@ void dbg_dump_node(const struct ubifs_info *c,
const void *node)
 		printk(KERN_DEBUG "\tflags          %#x\n", sup_flags);
 		printk(KERN_DEBUG "\t  big_lpt      %u\n",
 		       !!(sup_flags & UBIFS_FLG_BIGLPT));
+		printk(KERN_DEBUG "\t  leb_fixup    %u\n",
+		       !!(sup_flags & UBIFS_FLG_LEB_FIXUP));
 		printk(KERN_DEBUG "\tmin_io_size    %u\n",
 		       le32_to_cpu(sup->min_io_size));
 		printk(KERN_DEBUG "\tleb_size       %u\n",
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index bf31b47..1977ed2 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -80,7 +80,8 @@ static int create_default_filesystem(struct ubifs_info *c)
 	struct ubifs_cs_node *cs;
 	union ubifs_key key;
 	int err, tmp, jnl_lebs, log_lebs, max_buds, main_lebs, main_first;
-	int lpt_lebs, lpt_first, orph_lebs, big_lpt, ino_waste, sup_flags = 0;
+	int lpt_lebs, lpt_first, orph_lebs, big_lpt, leb_fixup, ino_waste;
+	int sup_flags = 0;
 	int min_leb_cnt = UBIFS_MIN_LEB_CNT;
 	long long tmp64, main_bytes;
 	__le64 tmp_le64;
@@ -155,6 +156,7 @@ static int create_default_filesystem(struct ubifs_info *c)
 		lpt_first + lpt_lebs - 1);

 	main_first = c->leb_cnt - main_lebs;
+	leb_fixup = 0; /* Normally no need to fixup a fresh [empty] FS */

 	/* Create default superblock */
 	tmp = ALIGN(UBIFS_SB_NODE_SZ, c->min_io_size);
@@ -165,6 +167,8 @@ static int create_default_filesystem(struct ubifs_info *c)
 	tmp64 = (long long)max_buds * c->leb_size;
 	if (big_lpt)
 		sup_flags |= UBIFS_FLG_BIGLPT;
+	if (leb_fixup)
+		sup_flags |= UBIFS_FLG_LEB_FIXUP;

 	sup->ch.node_type  = UBIFS_SB_NODE;
 	sup->key_hash      = UBIFS_KEY_HASH_R5;
@@ -616,6 +620,7 @@ int ubifs_read_superblock(struct ubifs_info *c)
 	c->vfs_sb->s_time_gran = le32_to_cpu(sup->time_gran);
 	memcpy(&c->uuid, &sup->uuid, 16);
 	c->big_lpt = !!(sup_flags & UBIFS_FLG_BIGLPT);
+	c->leb_fixup = !!(sup_flags & UBIFS_FLG_LEB_FIXUP);

 	/* Automatically increase file system size to the maximum size */
 	c->old_leb_cnt = c->leb_cnt;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 1f049ae..8345517 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1153,6 +1153,78 @@ static int check_free_space(struct ubifs_info *c)
 }

 /**
+ * ubifs_mount_fixup_lebs - erase empty pages on first mount (for NAND)
+ * @c: UBIFS file-system description object
+ *
+ * This function fixes up LEBs on first mount, if the appropriate flag was set
+ * when the FS was created.  Each LEB with trailing "empty" (all-0xff) pages
+ * is re-written, to make sure the empty space is actually erased.  This is
+ * necessary for some NAND chips, since the 0xff data may have been programmed
+ * like real data (generating a non-0xff ECC), causing future writes to the
+ * not-really-erased pages to behave badly.
+ */
+static int ubifs_mount_fixup_lebs(struct ubifs_info *c)
+{
+	int lnum, err = 0;
+	int sup_flags = 0;
+	struct ubifs_sb_node *sup;
+	struct ubifs_scan_leb *sleb;
+
+	ubifs_assert(c->leb_fixup);
+	ubifs_assert(!c->ro_mount);
+
+	ubifs_msg("LEB fixup needed");
+
+	ubifs_get_lprops(c);
+
+	/*
+	 * Try and "recover" any currently-mapped LEB, which will truly erase
+	 * any empty pages (they might currently exist as real 0xff data).
+	 */
+	for (lnum = 2; lnum <= c->leb_cnt; lnum++) {
+		lprops=ubifs_lpt_lookup(c, lnum);
+		if (IS_ERR(lprops)) {
+			err = PTR_ERR(lprops);
+			return err;
+		}
+
+		if (!(lprops->flags & LPROPS_EMPTY)) {
+			sleb = ubifs_recover_leb(c, lnum, 0, c->sbuf, 0);
+			if (IS_ERR(sleb)) {
+				err = PTR_ERR(sleb);
+				return err;
+			}
+			ubifs_scan_destroy(sleb);
+		}
+	}
+
+	ubifs_release_lprops(c);
+
+	sup = ubifs_read_sb_node(c);
+	if (IS_ERR(sup)) {
+		err = PTR_ERR(sup);
+		goto out;
+	}
+
+	/* LEB fixup is no longer required */
+	c->leb_fixup = 0;
+
+	/* Set new flags, omitting LEB fixup */
+	sup_flags = 0;
+	if (c->big_lpt)
+		sup_flags |= UBIFS_FLG_BIGLPT;
+	sup->flags = cpu_to_le32(sup_flags);
+
+	err = ubifs_write_sb_node(c, sup);
+	if (err)
+		goto out;
+
+	ubifs_msg("LEB fixup complete");
+out:
+	return err;
+}
+
+/**
  * mount_ubifs - mount UBIFS file-system.
  * @c: UBIFS file-system description object
  *
@@ -1396,6 +1468,12 @@ static int mount_ubifs(struct ubifs_info *c)
 	} else
 		ubifs_assert(c->lst.taken_empty_lebs > 0);

+	if (!c->ro_mount && c->leb_fixup) {
+		err = ubifs_mount_fixup_lebs(c);
+		if (err)
+			goto out_infos;
+	}
+
 	err = dbg_check_filesystem(c);
 	if (err)
 		goto out_infos;
@@ -1684,7 +1762,16 @@ static int ubifs_remount_rw(struct ubifs_info *c)
 		 * because, for example, the old index size was imprecise.
 		 */
 		err = dbg_check_space_info(c);
+		if (err)
+			goto out;
 	}
+
+	if (c->leb_fixup) {
+		err = ubifs_mount_fixup_lebs(c);
+		if (err)
+			goto out;
+	}
+
 	mutex_unlock(&c->umount_mutex);
 	return err;

diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index b922f03..dfffa5a 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -408,9 +408,11 @@ enum {
  * Superblock flags.
  *
  * UBIFS_FLG_BIGLPT: if "big" LPT model is used if set
+ * UBIFS_FLG_LEB_FIXUP: first-mount "fixup" of empty pages in LEBs needed
  */
 enum {
 	UBIFS_FLG_BIGLPT = 0x02,
+	UBIFS_FLG_LEB_FIXUP = 0x04,
 };

 /**
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 26a7ebe..c3d5544 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1254,6 +1254,7 @@ struct ubifs_info {
 	wait_queue_head_t cmt_wq;

 	unsigned int big_lpt:1;
+	unsigned int leb_fixup:1;
 	unsigned int no_chk_data_crc:1;
 	unsigned int bulk_read:1;
 	unsigned int default_compr:2;

-- 
Matthew L. Creech

Re: Programming ubinized images

[Artem Bityutskiy]

On Tue, 2011-05-03 at 02:43 -0400, Matthew L. Creech wrote:
> On Tue, Apr 26, 2011 at 4:00 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >
> > We could teach the UBIFS tools and the kernel to deal with these things.
> > It is possible to do with a special flag in the UBIFS superblock which
> > would say - this FS has been just flashed, do not use space in
> > half-filled eraseblocks! Then probably we could go through these
> > half-filled eraseblocks and fix them up, and then remove that flag.
> >
> 
> Hi Artem, could you take a look at this patch when you get a chance?
> 
> My method for finding & cleaning the in-use LEBs is hacky: I just loop
> through all the LEBs in the FS, look up each one in the LPT, and
> "recover" anything not flagged as empty.  I *think* that should result
> in LEBs that have trailing empty space (SCANNED_EMPTY_SPACE) being
> re-mapped to a new PEB (via fix_unclean_leb()), which would clean up
> the empty pages.  So far I haven't gotten that condition to trigger
> with a real UBIFS image, so I'm not sure if it works correctly yet.
> 
> I may be way off on the wrong track though, so I just wanted to see if
> you think this seems like a reasonable way of doing it.  Thanks!

Hi, very very quick comment. First of all, it should be a flag in the
superblock. Can you make a separate patch which just introduces this
flag there?

But yes, something like you've done: once LPT is initialized, you should
look at the flag and if it is set, indeed do something like you have
done - walk all lebs, get lprops decription, and then you see if there
is any free space. If there is not - do not touch it. If there is free
space, then fixup it.

The recovery function will work, but it dos much more than needed, and
it prints various messages which we do not want. Make a separate
function for fix-up in sb.c. It is easy - you simply read c->leb_size -
lp->free, and then you use ubi_leb_change() and change the contents of
the LEB in-place atomically.

And after the fix-up you need to clean the flag in the SB.

Just make a nice series of patches :-)


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)