* Re: [U-Boot] Bricked when trying to attach UBI
2012-12-20 16:02 ` [U-Boot] Bricked when trying to attach UBI Luca Ceresoli
@ 2013-01-02 14:37 ` Luca Ceresoli
0 siblings, 0 replies; 2+ messages in thread
From: Luca Ceresoli @ 2013-01-02 14:37 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Artem Bityutskiy, kmpark, Maxim Levitsky, Andreas Bießmann,
u-boot@lists.denx.de, Holger Brunck, Luca Ceresoli, linux-mtd,
Stefan Roese, Brian Norris
Luca Ceresoli wrote:
> Hi,
>
> I'm Cc'ing the linux-mtd list as well as the authors of the Linux
> commits cited below.
>
> For these new readers: I reported a problem with U-Boot 2012.04.01 not
> being able to attach an UBI partition in NAND, while Linux (2.6.37) can
> attach and repair it.
>
> It looks like an U-Boot bug, but I discovered strange things around the
> chip->badblockbits variable (in the NAND code) by comparing the
> relevant code in U-Boot and Linux.
>
> Sorry for Cc'ing so many people, but following this issue I was lead
> from one subsystem to another (and from U-Boot to Linux).
>
> Previous discussion is here:
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/149624
>
> Luca Ceresoli wrote:
>> Hi Andreas,
>>
>> Andreas Bießmann wrote:
>>> Hi Luca,
>>>
>>> On 19.12.2012 16:56, Luca Ceresoli wrote:
>>>> Hi Andreas,
>>>>
>>>> Andreas Bießmann wrote:
>>>> ...
>>>>>> Creating 1 MTD partitions on "nand0":
>>>>>> 0x000000100000-0x000010000000 : "mtd=3"
>>>>>> UBI: attaching mtd1 to ubi0
>>>>>> UBI: physical eraseblock size: 131072 bytes (128 KiB)
>>>>>> UBI: logical eraseblock size: 129024 bytes
>>>>>> UBI: smallest flash I/O unit: 2048
>>>>>> UBI: sub-page size: 512
>>>>>> UBI: VID header offset: 512 (aligned 512)
>>>>>> UBI: data offset: 2048
>>>>>> UBI error: ubi_wl_init_scan: no enough physical eraseblocks (0,
>>>>>> need 1)
>>>>>>
>>>>>> Now the device is totally blocked, and power cycling does not change
>>>>>> the result.
>>>>>
>>>>> have you tried to increase the malloc arena in u-boot
>>>>> (CONIG_SYS_MALLOC_LEN)?
>>>>> We had errors like this before [1],[2] and [3], maybe others -
>>>>> apparently with another error message, but please give it a try. We
>>>>> know
>>>>> ubi recovery needs some ram and 1MiB may be not enough.
>>>>
>>>> Thanks for your suggestion.
>>>>
>>>> Unfortunately this does not seem to be the cause of my problem: I
>>>> tried
>>>> increasing my CONFIG_SYS_MALLOC_LEN in include/configs/dig297.h from
>>>> (1024 << 10) to both (1024 << 12) and (1024 << 14), but without any
>>>> difference.
>>>
>>> Well, ok ... Malloc arena is always my first thought if I read about
>>> problems with ubi in u-boot.
>>> Have you looked up the differences in drivers/mtd/ubi/ in your u-boot
>>> and linux tree? Maybe you can see something obviously different in the
>>> ubi_wl_init_scan()?
>>
>> I had some days ago, but I double-checked now as you suggested. Indeed
>> there is an important difference: attach_by_scanning() (build.c) calls
>> ubi_wl_init_scan() and ubi_eba_init_scan() just like Linux does, but in
>> a swapped order!
>>
>> This swap dates back to:
>>
>> commit d63894654df72b010de2abb4b3f07d0d755f65b6
>> Author: Holger Brunck <holger.brunck@keymile.com>
>> Date: Mon Oct 10 13:08:19 2011 +0200
>>
>> UBI: init eba tables before wl when attaching a device
>>
>> This fixes that u-boot gets stuck when a bitflip was detected
>> during "ubi part <ubi_device>". If a bitflip was detected UBI tries
>> to copy the PEB to a different place. This needs that the eba table
>> are initialized, but this was done after the wear levelling worker
>> detects the bitflip. So changes the initialisation of these two
>> tasks in u-boot.
>>
>> This is a u-boot specific patch and not needed in the linux layer,
>> because due to commit 1b1f9a9d00447d
>> UBI: Ensure that "background thread" operations are really executed
>> we schedule these tasks in place and not as in linux after the
>> inital
>> task which schedule this new task is finished.
>>
>> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
>> cc: Stefan Roese <sr@denx.de>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>>
>> I tried reverting that commit and... surprise! U-Boot can now attach UBI
>> and boot properly!
>>
>> But the cited commit actually fixed a bug that bite our board a few
>> months back, so it should not be reverted without thinking twice. Now
>> it apparently introduced another bug. :-(
>>
>> I'm Cc:ing the commit author for comments.
>>
>> Nonetheless, I have evidence of a different behaviour between U-Boot
>> and Linux even before the two swapped functions are called.
>>
>> What attach_by_scanning() does in Linux is (abbreviated):
>>
>> static int attach_by_scanning(struct ubi_device *ubi)
>> {
>> si = ubi_scan(ubi);
>> ...fill ubi->some_fields...;
>> err = ubi_read_volume_table(ubi, si);
>> /* MARK */
>> err = ubi_eba_init_scan(ubi, si); /* swapped in U-Boot */
>> err = ubi_wl_init_scan(ubi, si); /* swapped in U-Boot */
>> ubi_scan_destroy_si(si);
>> return 0;
>> }
>>
>> See the two swapped calls.
>>
>> At MARK, I printed some of the peb counters in *ubi, and I got
>> different results for ubi->avail_pebs between U-Boot and Linux:
>> U-Boot: UBI: POST_TBL: rsvd=2018, avail=21, beb_rsvd_{pebs,level}=0,0
>> Linux: UBI: POST_TBL: rsvd=2018, avail=22, beb_rsvd_{pebs,level}=0,0
>>
>> The printed values were equal before calling ubi_read_volume_table().
>> I have no idea about where this difference comes from, nor if this
>> difference can cause my troubles.
>> I will better investigate tomorrow looking into ubi_read_volume_table().
>
> After half a day of debugging and an insane amount of printk()s added to
> both U-Boot and Linux, I have some more hints to understand the problem.
>
> The two different results quoted above show that U-Boot counted 21
> available eraseblocks, while Linux counts 22. I am not sure if this can
> cause my problem, but it's the first visible difference between U-Boot
> and Linux.
>
> This originates from ubi_scan() (scan.c): in U-Boot, it sets
> si->bad_peb_count to 1, in Linux to 0. U-Boot's ubi_scan() is very
> similar to Linux's, and the differences do not seem to relevant in my
> case. So let's dig down...
>
> - ubi_scan() (scan.c) calls process_eb() (scan.c) for each EB
> - process_eb() calls ubi_io_is_bad() (io.c), and if it returns >0 it
> increments si->bad_peb_count, which is what is happening to my board
> when executing U-Boot
> - ubi_io_is_bad() calls mtd->block_isbad(), which points to
> nand_block_isbad() (nand_base.c)
> - nand_block_isbad() is a wrapper to nand_block_checkbad() (nand_base.c)
> - nand_block_checkbad() differs from the Linux code in something
> related to lazy bad block scanning (commit fb49454b1b6c7c6, Feb 2012),
> but this does not seem to change the behaviour I observe;
> - nand_block_checkbad() calls either chip->block_bad() or
> nand_isbad_bbt(); I tracked only into the former, but I suspect the
> latter produces the same effects with regard to the problem I'm facing
> - chip->block_bad() points to nand_block_bad() (nand_base.c)
>
> nand_block_bad() (nand_base.c) does the following:
> static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
> {
> ...
>
> if (likely(chip->badblockbits == 8))
> res = bad != 0xFF;
> else
> res = hweight8(bad) < chip->badblockbits;
>
> if (getchip)
> nand_release_device(mtd);
>
> return res;
> }
>
> I don't understand the algorithm, but the relevant variables have these
> values:
> U-Boot: nand_block_bad: chip->badblockbits=8, bad=0000, hweight8(bad)=0
> Linux: nand_block_bad: chip->badblockbits=0, bad=0000, hweight8(bad)=0
> ^
>
> Obviously the U-Boot and Linux produce a different return value.
> This propagates up to ubi->bad_peb_count in ubi_scan(), and from there
> it changes the behaviour of the following code, leading to a block in
> U-Boot and a successful attach in Linux.
>
> chip->badblockbits in current Linux master is described as
> "minimum number of set bits in a good block's bad block marker
> position; i.e., BBM == 11110111b is not bad when badblockbits == 7".
>
> Still a bit obscure to me because I don't have a general picture.
> Anyway, here's how its value comes to be different between U-Boot
> (2012.04.01) and Linux (2.6.37).
>
> Linux:
> a) commit e0b58d0a7005, Feb 2010:
> mtd: nand: add ->badblockbits for minimum number of set bits in bad
> block byte
> declared the new variable and introduced in nand_get_flash_type()
> (nand_base.c) the following line:
> chip->badblockbits = 8;
> b) commit c7b28e25cb9, Jul 2010:
> mtd: nand: refactor BB marker detection
> removed from nand_get_flash_type() (nand_base.c) the same line:
> chip->badblockbits = 8;
> c) commit 26d9be11485e, Apr 2011:
> mtd: return badblockbits back
> restored in nand_get_flash_type() (nand_base.c) the following line:
> chip->badblockbits = 8;
> claiming it had been accidentally removed in commit b).
>
> The version of Linux I'm using (2.6.37), contains commits a) and b), so
> it has chip->badblockbits equal to 0. According to the log message of
> commit c), this should be wrong, but the resulting kernel works!
>
> The version of U-Boot (2012.04.01) contains the result of all 3 commits,
> since
>
> commit 2a8e0fc8b3dc31a3c571e439fbf04b882c8986be
> Author: Christian Hitz <christian.hitz@aizo.com>
> Date: Wed Oct 12 09:32:02 2011 +0200
>
> nand: Merge changes from Linux nand driver
>
> [backport from linux commit
> 02f8c6aee8df3cdc935e9bdd4f2d020306035dbe]
>
> This patch synchronizes the nand driver with the Linux 3.0 state.
>
> This looks like an improvement, but it bricks my board!
>
> I could not resist, and without even trying to understand what I was
> doing, I did in U-Boot's nand_get_flash_type() (nand_base.c):
>
> - chip->badblockbits = 8;
> + chip->badblockbits = 0;
>
> and guess what? U-Boot attached UBI, loaded Linux from it and booted
> successfully!
>
> No, I don't think changing lines here and there without any real
> understanding is a way to produce reliable software. But I'm unable
> to understand why the software that should work better actually bricks
> the board and the other one runs fine? And how do I know what the
> correct value for chip->badblockbits should be?
>
> And last but most important: how can I properly fix U-Boot?
I had another look at the commit that swapped the calls to
ubi_eba_init_scan() and ubi_wl_init_scan(), and I noticed that it
changed the
computationof the available PEB count.
In the original (pre-swap) code, running on a working board:
static int attach_by_scanning(struct ubi_device *ubi)
{
si = ubi_scan(ubi);
...fill ubi->some_fields...;
err = ubi_read_volume_table(ubi, si);
/* here rsvd=2018, avail=22, beb_rsvd_{pebs,level}=0,0 */
err = ubi_wl_init_scan(ubi, si); /* swapped in U-Boot */
/* herersvd=2019, avail=21, beb_rsvd_{pebs,level}=0,0 ***** */
err = ubi_eba_init_scan(ubi, si); /* swapped in U-Boot */
ubi_scan_destroy_si(si);
return 0;
}
In the current (post-swap) code, running on the same board:
static int attach_by_scanning(struct ubi_device *ubi)
{
si = ubi_scan(ubi);
...fill ubi->some_fields...;
err = ubi_read_volume_table(ubi, si);
/* here rsvd=2018, avail=22, beb_rsvd_{pebs,level}=0,0 */
err = ubi_eba_init_scan(ubi, si); /* swapped in U-Boot */
/* here rsvd=2039, avail=1, beb_rsvd_{pebs,level}=20,20***** */
err = ubi_wl_init_scan(ubi, si); /* swapped in U-Boot */
ubi_scan_destroy_si(si);
return 0;
}
Notice the difference on the line marked with "*****": after the swap, the
number of available PEBs changed from 21to 1.
According to the docs, UBI reserves some PEBs for bad PEB handling. By
default, in my 2048-PEBs NAND, it reserved 20 PEBs, wihch are far enough to
recover from a few bad PEBs. These should be computed as part of the
"available"
PEBs. But current U-Boot (incorrectly?) thinks thereis only 1 available PEB.
On a bricked board, it thinks there are 0, so it cannotattach UBI.
I have no fix for this, but I tried a simple workaround: instead of
using all
the available space for my logical volumes, I created them with a smaller
size, leaving 32 unused PEBs. Now, in attach_by_scanning(), I got:
pre-swap: rsvd=1987, avail=53, beb_rsvd_{pebs,level}=0,0
post-swap: rsvd=2007, avail=33, beb_rsvd_{pebs,level}=20,20
The computed number of available PEB is exactly 32 units bigger than it used
to be. This means, also after the swap, U-Boot thinks there are plenty of
available PEBs.
To try to simulate a board that has bad blocks, I then marked some blocks as
bad using 'nand markbad' in U-Boot. The number of available PEBs decreases
accordingly, but is still >0 and U-Boot can attach UBI and boot.
So, it seems that leaving some unused PEBs is a workaround to this problem!
I'm not 100% sure this is ok and will go on to better understand the
problem.
Any comments are welcome.
Luca
^ permalink raw reply [flat|nested] 2+ messages in thread