Erase error does not mark smth in BBT

Erase error does not mark smth in BBT

[Leon Pollak]

Hello, all.

I am studying the code in nand_base.c 
I thought that when erase command ends with an error, the corresponding 
block will be marked as bad or in OOB or in BBT or in both.
I think I walked through the code carefully, but did not find this 
BBT/OOB treatment.

Please, help me - where is my error?

BR
-- 
Leon

Re: Erase error does not mark smth in BBT

[Richard Weinberger]

On Sun, Dec 8, 2013 at 10:51 AM, Leon Pollak <leonp@plris.com> wrote:
> Hello, all.
>
> I am studying the code in nand_base.c
> I thought that when erase command ends with an error, the corresponding
> block will be marked as bad or in OOB or in BBT or in both.
> I think I walked through the code carefully, but did not find this
> BBT/OOB treatment.
>
> Please, help me - where is my error?

You're looking at the wrong layer. :-)
Look at the users of mtd_block_markbad().

-- 
Thanks,
//richard

Re: Erase error does not mark smth in BBT

[Leon Pollak]

On Sunday 08 December 2013 15:43:33 Richard Weinberger wrote:
> On Sun, Dec 8, 2013 at 10:51 AM, Leon Pollak <leonp@plris.com> wrote:
> > Hello, all.
> > I am studying the code in nand_base.c
> > I thought that when erase command ends with an error, the
> > corresponding block will be marked as bad or in OOB or in BBT or in
> > both.
> > I think I walked through the code carefully, but did not find this
> > BBT/OOB treatment.
> > 
> > Please, help me - where is my error?
> 
> You're looking at the wrong layer. :-)
> Look at the users of mtd_block_markbad().


Thank you very much, Richard.

Yes, I understood my problem - I expected to see the BBT update on 
erasure failure in the kernel driver.
But, as I understood after your hint and code study, the BBT update is 
totally on the "user's" responsibility.

---
So, I went to the mtd-utils package.
And there I have the similar problem - when looking into the 
flash_erase.c file I see the following:

if (mtd_erase(mtd_desc, &mtd, fd, eb) != 0) {
   sys_errmsg("%s: MTD Erase failure", mtd_device);
   continue;
}
and I was not able to find any usage of mtd_mark_bad.

Contrary, analyzing the nandwrite.c file, I found that the logic is as 
following (lines 537-555):
1. Try to write block data.
2. If failed, try to erase this block.
3. If erased failed (EIO) mark the block as bad and restart from (1).
---

So, here the question comes:
If the block erasure already failed once (in flash_erase.c) - doesn't 
this require the BBT update at this point?
Why do one need to retry to erase/program it?
AFAIK, most of NAND vendors state that the block once not erased is bad 
and even if you succeeded to erase it afterwords, it is dangerous to 
continue to use it.

Thank you very much for your help.
-- 
Leon

Re: Erase error does not mark smth in BBT

[Brian Norris]

On Mon, Dec 09, 2013 at 05:02:29PM +0200, Leon Pollak wrote:
> On Sunday 08 December 2013 15:43:33 Richard Weinberger wrote:
> > On Sun, Dec 8, 2013 at 10:51 AM, Leon Pollak <leonp@plris.com> wrote:
> > > I am studying the code in nand_base.c
> > > I thought that when erase command ends with an error, the
> > > corresponding block will be marked as bad or in OOB or in BBT or in
> > > both.
> > > I think I walked through the code carefully, but did not find this
> > > BBT/OOB treatment.
> > > 
> > > Please, help me - where is my error?
> > 
> > You're looking at the wrong layer. :-)
> > Look at the users of mtd_block_markbad().
> 
> Yes, I understood my problem - I expected to see the BBT update on 
> erasure failure in the kernel driver.
> But, as I understood after your hint and code study, the BBT update is 
> totally on the "user's" responsibility.
> 
> ---
> So, I went to the mtd-utils package.

The mtd_block_markbad() API is actually still a kernel-internal API, so
Richard was probably pointing you to the few in-kernel users, like UBI
(drivers/mtd/ubi/io.c) and other filesystems (e.g., JFFS2) or
translation layers (NFTL?).

The mtd-utils package may be a worse example, and they aren't actually
used for "regular wear" -- they are mostly for initial flashing and for
debugging. As you notice, flash_erase.c has some flaws (no bad block
marking). But the kernel UBI layer, the JFFS2 filesystem, the nandwrite
utility, and the user-space UBI utilities should be good examples.

> And there I have the similar problem - when looking into the 
> flash_erase.c file I see the following:
> 
> if (mtd_erase(mtd_desc, &mtd, fd, eb) != 0) {
>    sys_errmsg("%s: MTD Erase failure", mtd_device);
>    continue;
> }
> and I was not able to find any usage of mtd_mark_bad.

Correct, flash_erase.c does not automatically mark blocks bad. Perhaps
that would be a good flag to add, similar to nandwrite --markbad.

> Contrary, analyzing the nandwrite.c file, I found that the logic is as 
> following (lines 537-555):
> 1. Try to write block data.
> 2. If failed, try to erase this block.
> 3. If erased failed (EIO) mark the block as bad and restart from (1).

Your step 3 is wrong. If you're using the --markbad option, then this
step is just:

  3. If step (1) failed, mark the block as bad and continue from (1).

(Notice how we only error out if the erase in (2) fails for something
*besides* EIO.)

> ---
> 
> So, here the question comes:
> If the block erasure already failed once (in flash_erase.c) - doesn't 
> this require the BBT update at this point?

Yes, flash_erase.c probably should have the option to automatically mark
failed blocks as bad.

> Why do one need to retry to erase/program it?

nandwrite.c is a separate utility from flash_erase.c. nandwrite.c only
tries to erase a partially-failed write. But then it will still
unconditionally mark the block as bad. It is not retrying the
erase/program.

> AFAIK, most of NAND vendors state that the block once not erased is bad 
> and even if you succeeded to erase it afterwords, it is dangerous to 
> continue to use it.

Yes, very true. AFAICT, flash_erase.c is the only tool that is missing
this aspect, as it does not have a --markbad option.

Brian

Re: Erase error does not mark smth in BBT

[Richard Weinberger]

On Mon, Dec 9, 2013 at 7:44 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Mon, Dec 09, 2013 at 05:02:29PM +0200, Leon Pollak wrote:
>> On Sunday 08 December 2013 15:43:33 Richard Weinberger wrote:
>> > On Sun, Dec 8, 2013 at 10:51 AM, Leon Pollak <leonp@plris.com> wrote:
>> > > I am studying the code in nand_base.c
>> > > I thought that when erase command ends with an error, the
>> > > corresponding block will be marked as bad or in OOB or in BBT or in
>> > > both.
>> > > I think I walked through the code carefully, but did not find this
>> > > BBT/OOB treatment.
>> > >
>> > > Please, help me - where is my error?
>> >
>> > You're looking at the wrong layer. :-)
>> > Look at the users of mtd_block_markbad().
>>
>> Yes, I understood my problem - I expected to see the BBT update on
>> erasure failure in the kernel driver.
>> But, as I understood after your hint and code study, the BBT update is
>> totally on the "user's" responsibility.
>>
>> ---
>> So, I went to the mtd-utils package.
>
> The mtd_block_markbad() API is actually still a kernel-internal API, so
> Richard was probably pointing you to the few in-kernel users, like UBI
> (drivers/mtd/ubi/io.c) and other filesystems (e.g., JFFS2) or
> translation layers (NFTL?).

Correct. :)

-- 
Thanks,
//richard