
Hi,
On Tue, Jan 16, 2018 at 5:43 AM, Heiko Schocher hs@denx.de 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 hs@denx.de 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 mtownsend1973@gmail.com 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 mtownsend1973@gmail.com
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
#endif return_unused_pool_pebs(ubi, &ubi->fm_pool); return_unused_pool_pebs(ubi, &ubi->fm_wl_pool);update_fastmap_work_fn(ubi);
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: hs@denx.de
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: hs@denx.de