Hi, On Tue, Jan 16, 2018 at 5:43 AM, Heiko Schocher <[email protected]> wrote: > Hello Martin, > > added Richard to cc > > > Am 15.01.2018 um 13:13 schrieb Martin Townsend: >> >> Hi Heiko, >> >> >> On Mon, Jan 15, 2018 at 11:30 AM, Heiko Schocher <[email protected]> wrote: >>> >>> Hello Martin, >>> >>> >>> Am 12.01.2018 um 20:03 schrieb Martin Townsend: >>>>> >>>>> >>>>> From d35b7ea298fbd6c9d08b1b7132d43b9289d2b914 Mon Sep 17 00:00:00 2001 >>>> >>>> >>>> From: Martin Townsend <[email protected]> >>>> Date: Fri, 12 Jan 2018 18:59:23 +0000 >>>> Subject: [PATCH] ubi: Fix filesystem corruption on detach when fastmap >>>> enabled >>>> MIME-Version: 1.0 >>>> Content-Type: text/plain; charset=UTF-8 >>>> Content-Transfer-Encoding: 8bit >>>> >>>> When detaching using "ubi detach" it calls ubi_detach_mtd_dev which >>>> calls ubi_update_fastmap twice when fastmap is enabled. The second >>>> invocation was corrupting the ubifs as it was called after uif_close. >>>> Moved all calls to ubi_wl_close before uif_close. >>>> >>>> Signed-off-by: Martin Townsend <[email protected]> >>>> --- >>>> drivers/mtd/ubi/build.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c >>>> index baf4e2d..795ea34 100644 >>>> --- a/drivers/mtd/ubi/build.c >>>> +++ b/drivers/mtd/ubi/build.c >>>> @@ -1082,9 +1082,9 @@ out_debugfs: >>>> out_uif: >>>> get_device(&ubi->dev); >>>> ubi_assert(ref); >>>> + ubi_wl_close(ubi); >>>> uif_close(ubi); >>>> out_detach: >>>> - ubi_wl_close(ubi); >>>> ubi_free_internal_volumes(ubi); >>>> vfree(ubi->vtbl); >>>> out_free: >>>> @@ -1161,9 +1161,9 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway) >>>> get_device(&ubi->dev); >>>> >>>> ubi_debugfs_exit_dev(ubi); >>>> + ubi_wl_close(ubi); >>>> uif_close(ubi); >>>> >>>> - ubi_wl_close(ubi); >>>> ubi_free_internal_volumes(ubi); >>>> vfree(ubi->vtbl); >>>> put_mtd_device(ubi->mtd); >>>> >>> >>> Could you please try the following patch: >>> >>> diff --git a/drivers/mtd/ubi/fastmap-wl.c b/drivers/mtd/ubi/fastmap-wl.c >>> index a33d4063e0..2923d21836 100644 >>> --- a/drivers/mtd/ubi/fastmap-wl.c >>> +++ b/drivers/mtd/ubi/fastmap-wl.c >>> @@ -339,8 +339,6 @@ static void ubi_fastmap_close(struct ubi_device *ubi) >>> >>> #ifndef __UBOOT__ >>> flush_work(&ubi->fm_work); >>> -#else >>> - update_fastmap_work_fn(ubi); >>> #endif >>> return_unused_pool_pebs(ubi, &ubi->fm_pool); >>> return_unused_pool_pebs(ubi, &ubi->fm_wl_pool); >>> >>> Your problem is (I think) because U-Boot Code accidentially calls >>> update_fastmap_work_fn(ubi), but we do not need it here anymore, as >>> U-Boot does all UBI work immediately. >>> >>> bye, >>> Heiko >>> -- >>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >>> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected] >> >> >> That was my original fix so can confirm this also works. > > > Ok, great. > >> My reasoning for opting for the reordering was: I think the problem is >> uif_close frees up some UBI data structures so we have to ensure no >> updating of the filesystem occurs after this. What if >> ubi_fastmap_close or ubi_wl_close change in future and these changes >> result in updates to the filesystem, the same problem will occur and >> for our board it corrupts the UBIFS. So I opted to change the order in >> build.c. > > > Hmm... may Richard can comment here, because your change changes > code from linux, so if there is a potentiall problem, we should fix it > also in linux (or may this is fixed in mainline linux already?) > > BTW: your patch has checkpatch problems, see my weekly tbot tests: > > http://xeidos.ddns.net/tbot/id_590/tbot.txt > search for "2018-01-16 02:42" to see the wget command to get your patch > from patchwork > search for "2018-01-16 02:42:19" to see the checkpatch cmd output > > > [33mWARNING: [0m Possible unwrapped commit description (prefer a maximum 75 > chars per line) > #19: > Subject: [PATCH] ubi: Fix filesystem corruption on detach when fastmap > enabled > > [33mWARNING: [0m please, no spaces at the start of a line > #42: FILE: drivers/mtd/ubi/build.c:1085: > + ubi_wl_close(ubi);$ > > [33mWARNING: [0m please, no spaces at the start of a line > #53: FILE: drivers/mtd/ubi/build.c:1164: > + ubi_wl_close(ubi);$ > > Please fix this also in a v2, thanks! > > Huch, and search for "2018-01-16 02:42" ... your patch does not apply > to mainline U-Boot: > > 2018-01-16 02:42:20,650:CON :tbotlib # tb_ctrl: Applying: ubi: Fix > filesystem corruption on detach when fastmap enabled > Using index info to reconstruct a base tree... > error: patch failed: drivers/mtd/ubi/build.c:1082 > error: drivers/mtd/ubi/build.c: patch does not apply > error: Did you hand edit your patch? > It does not apply to blobs recorded in its index. > Patch failed at 0001 ubi: Fix filesystem corruption on detach when fastmap > enabled > The copy of the patch that failed is found in: .git/rebase-apply/patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". >
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. > > bye, > Heiko > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected] _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
