Re: [PATCH v2] MTD: modify mtd api to return bitflip info on read operations

[Mike Dunn <mikedunn@newsguy.com>]

On 12/06/2011 01:52 PM, Robert Jarzmik wrote:
> Mike Dunn <mikedunn@newsguy.com> writes:
>> On 12/05/2011 09:09 AM, Peter Horton wrote:
>>> Surely the check for "do we need to scrub ?" should be done lower down
>>> otherwise all users of the mtd NAND interface (UBI / JFFS2 etc) are going to
>>> have to re-implement those sysfs files and the corresponding checks.
>> Well, anything higher up that wants to avail itself of this api change will need
>> some rework regardless.  Currently the only info passed up from the driver is
>> that at least one bitflip occurred somewhere during the read.  The plan is to
>> eventually make some changes to UBI so that the decision to scrub is made more
>> intelligently.
> I'd like to second Mike's proposition here.
>
> Policy should not be put into drivers code whenever possible. The question "do
> we need to scrub" is to be answered in upper layers IMHO. Moreover, upper layer
> would then have the choice to trigger scrubbing on their own policy. This policy
> could rely on the information provided by the driver :
>  - how much bitflips the ECC can fix
>
> Even better, but I don't know how, the policy should be a userspace matter and
> not a kernel one, as I was told many times before :)
>
> Therefore, a upperlayer which strives at very high security level could scrub at
> 1 bitflip even if the ECC can fix 8. A "normal" upperlayer could go up to 8
> before scrubbing.


Yes, this flexibility is good, and why policy should not be set from below. 
Good point.  I just mentioned the "don't set policy" doctrine because I blindly
follow dogma :)


> Now for the "re-implement sysfs and checks", that's true. What could be done is
> in a first time, copycat the current behaviour, ie. UBI scrubs if nb_bitflips >
> 0, and the same for JFFS I suppose.
> Then in a second pass, add sysfs values for the trigger (whether in upperlayers
> or MTD). This needs a consensus from the list.


For UBI, the direction of the discussion was that the driver would indicate the
ecc strength of the device (via a new element in struct mtd_info), and this
would be the default UBI scrublevel.  A non-zero sysfs parameter would override
the default.

BTW, it is currently possible (and easy) to do what Peter suggests; i.e., to
avoid any UBI rework and make the decision to scrub in the driver.  All the
driver has to do is not return -EUCLEAN unless it thinks the block should be
scrubbed.  Of course this would be a lie if there were in fact corrected
bitflips.  For drivers using the mtd nand interface, the lie would be in the ecc
stats, since the nand infrastructure code returns -EUCLEAN (or 0) based on
that.  This is what we willl have to do in the diskonchip drivers in order to
get ubi/ubifs to work on them if we don't change the mtd api (and afterwards
make the changes to ubi).

Thanks,
Mike