Re: [PATCH 4.9 069/101] ubi: fastmap: Correctly handle interrupted erasures in EBA

[Lars Persson]

On Sun, Sep 23, 2018 at 2:58 PM Richard Weinberger <richard@xxxxxx> wrote:
>
> Lars,
>
> Am Sonntag, 23. September 2018, 14:49:23 CEST schrieb Lars Persson:
> > On Sun, Jul 1, 2018 at 6:27 PM Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > 4.9-stable review patch.  If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Richard Weinberger <richard@xxxxxx>
> > >
> > > commit 781932375ffc6411713ee0926ccae8596ed0261c upstream.
> > >
> > > Fastmap cannot track the LEB unmap operation, therefore it can
> > > happen that after an interrupted erasure the mapping still looks
> > > good from Fastmap's point of view, while reading from the PEB will
> > > cause an ECC error and confuses the upper layer.
> > >
> > > Instead of teaching users of UBI how to deal with that, we read back
> > > the VID header and check for errors. If the PEB is empty or shows ECC
> > > errors we fixup the mapping and schedule the PEB for erasure.
> > >
> > > Fixes: dbb7d2a88d2a ("UBI: Add fastmap core")
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > Reported-by: martin bayern <Martinbayern@xxxxxxxxxxx>
> > > Signed-off-by: Richard Weinberger <richard@xxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > >
> > >
> > Hi Greg
> >
> > This commit belongs to a series of 3 commits that are intended to be
> > used together. Currently the stable branches have only the first
> > commit from the series and we get a UBI speed regression because an
> > extra NAND page read is always performed for each access to a UBI LEB.
> >
> > 3e5e4335cc0ffd668054564b113fb3c9c97badb8 ubi: fastmap: Detect EBA
> > mismatches on-the-fly
> > 34653fd8c46e771585fce5975e4243f8fd401914 ubi: fastmap: Check each
> > mapping only once
> > 781932375ffc6411713ee0926ccae8596ed0261c ubi: fastmap: Correctly
> > handle interrupted erasures in EBA
> >
> > This will in turn require also this follow-up patch:
> > 25677478474a91fa1b46f19a4a591a9848bca6fb ubi: Initialize Fastmap
> > checkmapping correctly
>
> Wait.
> Commit 34653fd8c46e771585fce5975e4243f8fd401914 was not scheduled for stable
> on purpose.
> It is just an optimization. How much is the performance regression you see?
> Commit 3e5e4335cc0ffd668054564b113fb3c9c97badb8 does not fix anything, all it
> does is adding another paranoia check to UBI.
>
> I'd appreciate if you would come up with regression reports on linux-mtd first
> before asking Greg to pick patches up...
>

Hi Richard

Sorry, I assumed this omission from -stable was a mistake.

The timing for our boot increased from 45 seconds to 55 seconds on one
chip and 42 seconds to 48 seconds on another chip. The regression was
completely fixed by applying the extra patches. The way I see it the
first patch is a significant slow-down so the second patch is required
to restore performance.

- Lars