lkml.org 
[lkml]   [2018]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 02/14] ubi: Fix assert in ubi_wl_init
On Wed, 13 Jun 2018 23:23:32 +0200
Richard Weinberger <richard@nod.at> wrote:

> When multiple PEBs are used for a fastmap, found_pebs
> can be wrong and the assert triggers.
>
> The current approach is broken in two ways:
> 1. The "continue" in list_for_each_entry() over all fastmap PEBs
> misses the counter at all.
> 2. When the fastmap changes in size, growing due to a preseeded fastmap
> or shrinking due to new bad blocks, iterating over the current
> fastmap will give wrong numbers. We have to exclude the fastmap size
> at all from the calculation to be able to compare the numbers.
> At this stage we simply have no longer the information whether the
> fastmap changed in size.

Should this patch be backported to stable releases? You say the problem
arises when new bad blocks appear, so it's not only a problem you'll
have with the preseeded fastmap changes.

>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> drivers/mtd/ubi/wl.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
> index f66b3b22f328..6bbb968fe9da 100644
> --- a/drivers/mtd/ubi/wl.c
> +++ b/drivers/mtd/ubi/wl.c
> @@ -1695,11 +1695,19 @@ int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai)
> err = erase_aeb(ubi, aeb, sync);
> if (err)
> goto out_free;
> - }
>
> - found_pebs++;
> + /*
> + * If no fastmap is used, all fastmap PEBs will get be

remove either "get" or "be" here ^

> + * erased and are member of ai->fastmap.
> + */
> + if (!ubi->fm)
> + found_pebs++;
> + }
> }
>
> + if (ubi->fm)
> + found_pebs += ubi->fm->used_blocks;
> +

Hm, are we sure this is always correct? I mean, what if you had an old
fastmap scheduled for erasure but a power-cut happened before it was
erased. Are you sure we won't have an inconsistent found_pebs number
(found_pebs != ubi->good_peb_count).

I understand that this problem already exists because of

if (ubi->lookuptbl[aeb->pnum])
continue;

but I'm not sure your solution fixes that.

> dbg_wl("found %i PEBs", found_pebs);
>
> ubi_assert(ubi->good_peb_count == found_pebs);

\
 
 \ /
  Last update: 2018-06-20 12:11    [W:0.113 / U:23.424 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site