Re: [PATCH] ubi: do not re-read the data already read out in retry

[Brian Norris <computersforpeace@gmail.com>]

On Mon, Aug 24, 2015 at 09:05:12AM +0800, Dongsheng Yang wrote:
> On 08/21/2015 05:17 PM, Richard Weinberger wrote:
> >Brian, does MTD core guarantee that upon an erroneous mtd_read() the number of "read" bytes
> >in "buf" are valid?

I'll take a look. I believe the Dongsheng is interpreting the mtd_read()
API correctly, but most MTD "guarantees" are more like surveys of
existing practice; most implementations may do something sane, but there
may be a few oddballs.

> >So, my fear is that this change will introduce new regressions (due faulty MTD drivers, etc..)
> >without a real gain.
> 
> I would say "big gain" rather than "real gain".

I'd like evidence, rather than just bold statements :)

> Consider this case, if
> you are going to read 4M from driver but it failed at the last byte
> twice. When we success on the third retry, we have read out 4*3=12M for
> 4M data user requested. That tripled the latency.

On what driver do you see this? Many drivers take an all-or-nothing
approach already; they don't fill in 'retlen' to any non-zero value
until the last byte, so Richard is right in those cases.

(I can partly answer this question; nand_do_read_ops() incrementally
fills pages, and *retlen increases when a (portion of) a page is read
correctly.)

Also, what sort of failure modes are you seeing? It seems a bit dubious
to be relying on UBI I/O retry, let alone optimizing for performance,
since the retry may be masking significant problems anyway.

> But with this patch applied. we do not re-try the data we have already
> read out. So the latency is 1/3 of it mentioned above.
> 
> But, yes, I said understandable, I also want Brian to say is this
> change safe enough currently.

Brian