| Message ID | 20180613212344.11608-11-richard@nod.at |
|---|---|
| State | Superseded |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | ubi: Fastmap updates | expand |
On Wed, 13 Jun 2018 23:23:40 +0200 Richard Weinberger <richard@nod.at> wrote: > This WARN_ON() was used while development of fastmap, now > it can be a regular ubi_err(). > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/mtd/ubi/fastmap.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c > index dabeb01af24a..d5506152c9f7 100644 > --- a/drivers/mtd/ubi/fastmap.c > +++ b/drivers/mtd/ubi/fastmap.c > @@ -949,14 +949,16 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, > ubi_assert(list_empty(&free)); > > /* > - * If fastmap is leaking PEBs (must not happen), raise a > - * fat warning and fall back to scanning mode. > - * We do this here because in ubi_wl_init() it's too late > - * and we cannot fall back to scanning. > + * The orginal purpose of this check was detecting fastmap bugs were > + * it missed blocks. Now it helps to detect mis-flashed UBIs. > + * E.g. when a fastmap enabled UBI is copied to another target with > + * different bad blocks. > */ > - if (WARN_ON(count_fastmap_pebs(ai) != ubi->peb_count - > - ai->bad_peb_count - fm->used_blocks)) > + if (count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count - > + fm->used_blocks) { Nitpick, but can you align things like that: if (count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count - fm->used_blocks) { or if (count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count - fm->used_blocks) { or even better, have an avail_peb_count variable: avail_peb_count = ubi->peb_count - ai->bad_peb_count - fm->used_blocks; if (count_fastmap_pebs(ai) != avail_peb_count) > + ubi_err(ubi, "number of PEBs referenced by fastmap does not match MTD!"); > goto fail_bad; Also, I'm not sure I get it. Why is that an error now that we have preseeded fastmap? Isn't it expected to have a potential mismatch the first time we boot, or is it already taken into account before we reach this point? > + } > > return 0; >
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c index dabeb01af24a..d5506152c9f7 100644 --- a/drivers/mtd/ubi/fastmap.c +++ b/drivers/mtd/ubi/fastmap.c @@ -949,14 +949,16 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, ubi_assert(list_empty(&free)); /* - * If fastmap is leaking PEBs (must not happen), raise a - * fat warning and fall back to scanning mode. - * We do this here because in ubi_wl_init() it's too late - * and we cannot fall back to scanning. + * The orginal purpose of this check was detecting fastmap bugs were + * it missed blocks. Now it helps to detect mis-flashed UBIs. + * E.g. when a fastmap enabled UBI is copied to another target with + * different bad blocks. */ - if (WARN_ON(count_fastmap_pebs(ai) != ubi->peb_count - - ai->bad_peb_count - fm->used_blocks)) + if (count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count - + fm->used_blocks) { + ubi_err(ubi, "number of PEBs referenced by fastmap does not match MTD!"); goto fail_bad; + } return 0;
This WARN_ON() was used while development of fastmap, now it can be a regular ubi_err(). Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/fastmap.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)