Re: Regarding latest EUCLEAN/bitflip_threshold patchset

[Mike Dunn <mikedunn@newsguy.com>]

On 05/11/2012 04:51 AM, Artem Bityutskiy wrote:
> On Wed, 2012-05-09 at 13:26 +0300, Shmulik Ladkani wrote:
>> Hi Mike,
>>
>> I noticed 'nand_do_read_oob' explicitly tests 'mtd->ecc_stats.corrected'
>> and returns EUCLEAN - as it was not ported to use the new
>> 'bitflip_threshold'.
> 
> Good point. Mike did you miss this function?


No, I deliberately left it as-is.  Maybe a litle lazy or sloppy on my part, but
ignoring it is of no consequence (see below).

Shmulik, your posts still don't get to my inbox.  Still wondering why.  Anyway,
pasting from the archive...


> From nand_base.c:
>
> 	if (mtd->ecc_stats.failed - stats.failed)
> 		return -EBADMSG;
>
> 	return  mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
>
> - May drivers increment mtd->ecc_stats.{corrected,failed} during their
>   ecc.read_oob() call?


Currently no nand drivers increment stats.corrected for oob-only reads.  Since
nand_do_read_oob() does not read page data, stats never increment and -EUCLEAN
is never returned.  To avoid complicating the issue, I ignored the case of
reading oob-only.


> - If so, can we (should we?) report EUCLEAN according to the
>   bitflip_threshold in this case?


I guess it depends on how widespread is the desire or capability of performing
ecc on oob-only reads.  The new diskonchip devices (docg3, docg4) are capable of
performing ecc on oob-only data.  These can do one bit corrections over 15 (of
the 16 total) oob bytes using the hamming algorithm (though neither driver
supports it currently).  But since in this case only one bitflip can be
corrected, it will always be below bitflip_threshold.  Then there's the question
of how do you interpret uncorrectible bitflips vis-a-vis eraseblock health when
using a weaker ecc algorithm for oob-only.

These questions are currently all theoretical.  I think the threshold test
should be removed, and replaced with 'return 0', at least for now.

Thanks,
Mike