Re: mtd nand erase and bad block

[Shmulik Ladkani <shmulik.ladkani@gmail.com>]

Hi Angus,

On Fri, 01 Jun 2012 15:03:19 +0100 Angus CLARK <angus.clark@st.com> wrote:
> Hi Shmulik,
> 
> >>>
> >>> Typically, debugfs is only enabled in development environments, and even then it
> >>> requires explicit user action, so this method of enabling erasing bad blocks is
> >>> safe enough for our needs.
> >>
> >> Sounds ok to me, especially if you send the patch together with a piece
> >> of doc for the mtd web-site. I just think it is important to document
> >> this feature. Is this doable?
> > 
> > I think we should prefer a local "allow erase bad blocks" policy than a
> > global one.
> > 
> > This is because when the global debugfs flag is on, *every* mtd erase
> > operation might lead to erasure of bad blocks - not necessarily those
> > triggered by the user which set the flag prior issuing his 'flash_erase'
> > command.
> > 
> > Meaning, other MTD users (ubi, various ffs) which currently work on
> > other mtd partitions, are suddenly relaxed and allowed to erase bad
> > blocks - which is probably not what user intented.
> > 
> > I suggest to be more restrictive and have the "allow erase bad blocks"
> > propery be local policy, that is - per an erase request.
> > 
> > And since we'll probably need this thing only for userspace erase
> > calls (e.g. flash_erase) - I suggest placing it into the MEMERASE ioctl.
> > 
> > Comments?
> > 
> 
> I would agree to some extent.  Enabling the "allow erase bad blocks" option per
> erase request would certainly be a safer solution.  However, I suspect extending
> the existing MEMERASE/MEMERASE64 IOCTLs is not really an option.  That leaves
> inventing another IOCTL, or perhaps adding another file mode, which would
> achieve per-file-descriptor scope, if not per-erase-request.
> 
> My approach was largely motivated by the desire not to change the existing ABI,
> and/or mtd-utils.  One could argue that such an option should only ever be
> enabled by someone who knows what they are doing, and that might include things
> like un-mounting any filesystems beforehand.
> 
> To be honest, I am in two minds.  The solution I have at present is very simple
> (or perhaps naive!), requires minimial changes to the kernel, no changes to
> mtd-utils, and can be disabled completely by not including debugfs (which is
> standard practice on production systems).  On the other hand, the ability to
> enable per-erase-request is a safer and more elegant solution.  However, it
> would require updates to mtd-utils, and agreement from the MTD community
> regarding changes to the ABI...
> 
> What do you think?

To be honest, I'm in two minds either ;)

I completely understand the reasons and motivation for a global
debugfs option. And it seems as a reasonable compromise.

OTOH, adding a new ioctl makes sense as we're offering a functionality
that didn't exist before.

Anyways, I guess its up to David or Artem.

My personal preference would be:
1. A new ioctl (MEMSCRUB?)
2. debugfs flag, PER MTD PART (slightly safer than your global flag)
3. global debugfs flag

BTW what do you think about option (2)? Would you consider it, or do you
think it's an overdesign, if we already accept the debugfs way?

Regards,
Shmulik