Re: linux equivalent of u-boot's "nand scrub" (erasing blocks even when OOB says "bad")

[Artem Bityutskiy <dedekind1@gmail.com>]

On Wed, 2010-09-22 at 03:43 -0400, Mike Frysinger wrote:
> On Sunday, September 12, 2010 03:54:03 Artem Bityutskiy wrote:
> > On Sun, 2010-09-12 at 00:03 -0400, Mike Frysinger wrote:
> > > On Sat, Sep 11, 2010 at 02:32, Artem Bityutskiy wrote:
> > > > It will be confusing if the same word is used in MTD for "unmarking"
> > > > eraseblocks. How about: 'force erase' or 'bad erase' ?
> > > 
> > > that makes it sound like an option to the existing MEMERASE operation.
> > > 
> > >  so i guess what if we just do that -- extend the erase_info_user
> > > 
> > > structure to contain a flags field and add a MEMERASE2 that works with
> > > the larger structure ?  for now we'd only have one option (FORCE), but
> > > it makes it easy to extend in the future.
> > 
> > Ohh, this was so stupid of me to not ask people to add extra fields to
> > 'struct erase_info_user64' which was introduced relatively recently... I
> > always add extra fields to ioctl data structures...
> > 
> > But yeah, what you say sounds ok to me.
> 
> here's a POC that works for me.  with a simple tweak to `flash_eraseall`, i
> can now recover my mtd devices with funky OOB layouts.
> 
> ive only extended MEMERASE64 as i believe the non-64 variants are EOL ?  or
> should i also extend the 32bit interface as well ?

We need something consistent. This patch will just erase the bad
eraseblock. This will not mark it as good in the BBT (neither in-ram nor
on-flash). If the erasure succeeds, the block will still be marked as
bad in BBT, but after reboot, if the BBT is not on-flash, it will be
treated as good eraseblock, because scanning will not find the bad block
marker anymore. If the BBT is on-flash, it'll stay bad. This is
inconsistent.

> +struct erase_info_user64_flags {
> +	__u64 start;
> +	__u64 length;
> +	__u32 flags;
> +};

I think it needs to have som more room for possible future extentions.
Also, good tone for ioctls is to make them to be multiple of 64-bit -
less pain in mixed 32/64 bit setups.

Please, add some

u8 padding[12]

field and add a comment that this has to be zero, and may be used in
future.

Then in future we may extend ioctls and add more fields.

> +#define MEMERASE64_FLAGS	_IOW('M', 23, struct erase_info_user64_flags)

I do not like the name. We may add something else, not just flags later.
May be MEMERASE64_EXTENDED ?

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