[U-Boot] [PATCH] ubi: Fix filesystem corruption on detach when fastmap enabled

[Martin Townsend]

Hi Richard,

On Tue, Jan 16, 2018 at 1:22 PM, Richard Weinberger
<richard at sigma-star.at> wrote:
> Heiko, Martin,
>
> Am Dienstag, 16. Januar 2018, 10:11:41 CET schrieb Martin Townsend:
>> Ah. Must be the mail client. Sorry about that I'll setup git send-mail
>> for v2.  I'll wait to see what Richard has to say in case there is a
>> better fix.
>
> Thanks for letting me know!
> Indeed, there is a problem. I'm a little astonished that nobody noticed so
> far.
> Most likely because while detaching UBI in Linux mostly no Fastmap work is
> scheduled,
> and therefore in most cases flush_work(&ubi->fm_work) does nothing.
>
> As you noticed in U-Boot the story is different, you always update the Fastmap
> upon detach.
>
> Martin, can you please explain what corruption you see?
> From reading the code I'd assume that you miss volumes but a full scan would
> recover everything.

I didn't do much analysis of the corruption I'm afraid.  When I tried
to reattach the the c->empty flag was set to true and indeed all the
EBA tables were reporting an empty filesystem even after a reboot.
Not sure if this was recoverable, how do you trigger a full scan?

>
> For Linux I suggest this fix:
>
> From 48287459cf8717cffac5aed423937cd7ba4360ab Mon Sep 17 00:00:00 2001
> From: Richard Weinberger <richard at nod.at>
> Date: Tue, 16 Jan 2018 14:12:46 +0100
> Subject: [PATCH] ubi: fastmap: Don't flush fastmap work on detach
>
> At this point UBI volumes have already been free()'ed and fastmap can no
> longer access these data structures.
>
> Fixes: 74cdaf24004a ("UBI: Fastmap: Fix memory leaks while closing the WL sub-
> system")
> Signed-off-by: Richard Weinberger <richard at nod.at>
> Cc: stable at vger.kernel.org
> ---
>  drivers/mtd/ubi/fastmap-wl.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/ubi/fastmap-wl.c b/drivers/mtd/ubi/fastmap-wl.c
> index 4f0bd6b4422a..69dd21679a30 100644
> --- a/drivers/mtd/ubi/fastmap-wl.c
> +++ b/drivers/mtd/ubi/fastmap-wl.c
> @@ -362,7 +362,6 @@ static void ubi_fastmap_close(struct ubi_device *ubi)
>  {
>         int i;
>
> -       flush_work(&ubi->fm_work);
>         return_unused_pool_pebs(ubi, &ubi->fm_pool);
>         return_unused_pool_pebs(ubi, &ubi->fm_wl_pool);
>
> --
> 2.13.6
>
> In U-Boot you can do the same.
>
> Thanks,
> //richard
>
> --
> sigma star gmbh - Eduard-Bodem-Gasse 6 - 6020 Innsbruck - Austria
> ATU66964118 - FN 374287y

Richard: This will work but just want to check that ubi_wl_close is
supposed to never write out to the filesystem or will never do so in
future, if so maybe a something in the comments above ubi_wl_close to
ensure this?