Re: [PATCH 0/4] MTD: Change meaning of -EUCLEAN return code on reads

[Artem Bityutskiy <dedekind1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

Hi,

On Sun, 2012-03-11 at 14:21 -0700, Mike Dunn wrote:
> This problem was discussed a while back [1][2][3], and a consensus of sorts was
> reached for a solution, which these patches implement.  The recent addition of
> the mtd api entry functions now make this solution practical (thanks Artem!).  A
> quick description of each patch will provide a synopsis of the patchset:
> 
> (1) The element 'ecc_strength' is added to struct mtd_info, which will store the
>     maximum number of bit errors that can be corrected in one writesize region.

I think this attribute should be exported via sysfs as R/O.

> (2) Drivers set ecc_strength during initialization.
> 
> (3) The element 'euclean_threshold' is added to struct mtd_info.  If the driver
>     leaves this uninitialized, mtd sets it to ecc_strength when the device or
>     partition is registered.  This element is also exposed for reading and
>     writing from userspace through sysfs.

Would you please rename it to bitflip_threshold. It is bearable when it
is just a  'struct mtd_info' member, but you also export
'euclean_threshold' sysfs file which is really confusing from user
perspective, I think.

> (4) The drivers' read methods, absent an error, return a non-negative integer
>     indicating the maximum number of bit errors that were corrected in any one
>     writesize region.  MTD returns -EUCLEAN if this is >= euclean_threshold, 0
>     otherwise.
> 
> So basically, the meaning of -EUCLEAN is changed from "one or more bit errors
> were corrected over the entire read operation", to "a dangerously high number of
> bit errors were corrected on one or more writesize regions".  By default,
> "dangerously high" is interpreted as the maximum number of correctible bit
> errors per writesize.  Drivers can specify a different value, and the user can
> override it if more or less caution regarding data integrity is desired.

Please, make sure we have a good comment like this in the mtd.h file as
well. I think the one you added is not verbose enough.

Otherwise looks good!

> Patch #2 touches a lot of files, but they are small changes in most cases.  If
> you can verify the correctness of the device's ecc strength, an ACK would be
> much appreciated!

I'd be pro-active and just CC'ed maintainers/possibly relevant people.
scripts/get_maintainer.pl would give you the ones, I think. Just spend a
little time and come up with a list of people and CC them.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]