UBI: fix rb_tree node comparison in add_map commit buggy?
Richard Weinberger
richard at nod.at
Mon Jun 23 04:58:27 PDT 2014
Hi!
Am 23.06.2014 13:04, schrieb Heiko Schocher:
> Hello all,
>
> I just ported the linux v3.14 MTD/UBI/UBIFS sources to U-Boot to
> get in U-Boot also UBI fastmap support. This works now nice in
> U-Boot with linux v3.14 kernels :-)
>
> Now just booted a linux v3.16-rc1 kernel and I could not mount the
> ubifs based rootfs anymore (using UBI Fastmap support active and
> burned the ubi rootfs image in U-Boot) ...
>
> I found this patch in the linux tree:
>
> commit 604b592e6fd3c98f21435e1181ba7723ffc24715
> Author: Mike Snitzer <snitzer at redhat.com>
> Date: Fri Mar 21 15:54:03 2014 -0400
>
> UBI: fix rb_tree node comparison in add_map
>
> The comparisons used in add_vol() shouldn't be identical. Pretty sure
> the following is correct but it is completely untested.
>
> Signed-off-by: Mike Snitzer <snitzer at redhat.com>
> Acked-by: Richard Weinberger <richard at nod.at>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy at linux.intel.com>
>
> Reverted it in the linux tree, and my rootfs boots again ...
> (This patch is not yet in the u-boot sources, as they based on v3.14)
>
> So, this change seems at least to break backwardcompatibility... (I
> must admit, I am not a ubi nor ubi fastmap expert ...)
>
> I tried this patch also in the U-Boot code, without getting a ubifs
> rootfs (burned with u-boot into the ubi volume) bootable in linux ...
> which worked before... maybe v3.14 and v3.16-rc1 are not compatible?
> Do anybody know from such problems?
No. If there is a problem it needs fixing. :)
> If I revert commit 604b592e6fd3c98f21435e1181ba7723ffc24715 in
> Linux v3.16-rc1 only (so I have the same code in U-Boot at this
> place), I can boot Linux with the ubifs based rootfs, and after
> a reboot the U-Boot ubi attach time is short ... all works fine,
> as I can see it ...
So, Linux can no longer attach UBI Fastmap images which are older than 3.16-rc1?
How odd. :(
Attaching newer ones works?
> If I make in U-Boot and Linux in following patch [1]:
> $ git diff
> diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
> index b04e7d0..72f39da 100644
> --- a/drivers/mtd/ubi/fastmap.c
> +++ b/drivers/mtd/ubi/fastmap.c
> @@ -125,7 +125,7 @@ static struct ubi_ainf_volume *add_vol(struct ubi_attach_info *ai, int vol_id,
> parent = *p;
> av = rb_entry(parent, struct ubi_ainf_volume, rb);
>
> - if (vol_id < av->vol_id)
> + if (vol_id > av->vol_id)
> p = &(*p)->rb_left;
> else
> p = &(*p)->rb_right;
> $
>
> I can boot the ubifs based rootfs (burned with U-Boot), and after reboot
> to U-Boot U-Boot ubi attach works again fine.
>
> As in process_pool_aeb() looks:
> [...]
> if (be32_to_cpu(new_vh->vol_id) > tmp_av->vol_id)
> p = &(*p)->rb_left;
> else if (be32_to_cpu(new_vh->vol_id) < tmp_av->vol_id)
> p = &(*p)->rb_right;
>
> so, I think the old code:
>
> if (vol_id > av->vol_id)
> p = &(*p)->rb_left;
> else if (vol_id > av->vol_id)
> p = &(*p)->rb_right;
>
> is an copy&paste error ... and I think the right fix should
> be[1] ...
Forgot to attach it?
Thanks,
//richard
More information about the linux-mtd
mailing list