* [PATCH] mtd: ubi: Fix endless loop when moving PEB @ 2016-06-22 9:01 Teresa Remmet 2016-06-24 11:20 ` Sascha Hauer 0 siblings, 1 reply; 4+ messages in thread From: Teresa Remmet @ 2016-06-22 9:01 UTC (permalink / raw) To: barebox When moving a PEB the leb_write_trylock() function is called. As the function never returns 0 UBI will end up in an endless loop. Noticed the issue when fastmap has been enabled and data is beeing copied several times to a UBI volume. When UBI tries to move the anchor PEB, the issue comes up. The leb_write_trylock() is now equal to the leb_write_lock(). But kept it for easier maintaince in future when syncing with kernel. Signed-off-by: Teresa Remmet <t.remmet@phytec.de> --- drivers/mtd/ubi/eba.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c index a7af247..31dbcd2 100644 --- a/drivers/mtd/ubi/eba.c +++ b/drivers/mtd/ubi/eba.c @@ -257,16 +257,7 @@ static int leb_write_trylock(struct ubi_device *ubi, int vol_id, int lnum) le = ltree_add_entry(ubi, vol_id, lnum); if (IS_ERR(le)) return PTR_ERR(le); - - /* Contention, cancel */ - le->users -= 1; - ubi_assert(le->users >= 0); - if (le->users == 0) { - rb_erase(&le->rb, &ubi->ltree); - kfree(le); - } - - return 1; + return 0; } /** -- 1.9.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: ubi: Fix endless loop when moving PEB 2016-06-22 9:01 [PATCH] mtd: ubi: Fix endless loop when moving PEB Teresa Remmet @ 2016-06-24 11:20 ` Sascha Hauer 2016-06-27 8:22 ` Teresa Remmet 0 siblings, 1 reply; 4+ messages in thread From: Sascha Hauer @ 2016-06-24 11:20 UTC (permalink / raw) To: Teresa Remmet; +Cc: barebox Hi Teresa, On Wed, Jun 22, 2016 at 11:01:57AM +0200, Teresa Remmet wrote: > When moving a PEB the leb_write_trylock() function is called. > As the function never returns 0 UBI will end up in an endless > loop. > > Noticed the issue when fastmap has been enabled and data is beeing copied > several times to a UBI volume. When UBI tries to move the anchor PEB, > the issue comes up. > > The leb_write_trylock() is now equal to the leb_write_lock(). > But kept it for easier maintaince in future when syncing with > kernel. > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de> > --- > drivers/mtd/ubi/eba.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c > index a7af247..31dbcd2 100644 > --- a/drivers/mtd/ubi/eba.c > +++ b/drivers/mtd/ubi/eba.c > @@ -257,16 +257,7 @@ static int leb_write_trylock(struct ubi_device *ubi, int vol_id, int lnum) > le = ltree_add_entry(ubi, vol_id, lnum); > if (IS_ERR(le)) > return PTR_ERR(le); > - > - /* Contention, cancel */ > - le->users -= 1; > - ubi_assert(le->users >= 0); > - if (le->users == 0) { > - rb_erase(&le->rb, &ubi->ltree); > - kfree(le); > - } > - > - return 1; > + return 0; > } This seems to be the missing link to make the following patch work. I have tried this several times but so far never found this bug. Could you give it a try? With this patch it should be now possible to ubiformat a device with an image and to generate a fastmap when the device is detached first. Sascha ------------------------8<--------------------------------- From 94cea61ade69f712383e282d6aa573d3a12f11d0 Mon Sep 17 00:00:00 2001 From: Sascha Hauer <s.hauer@pengutronix.de> Date: Fri, 24 Jun 2016 11:38:24 +0200 Subject: [PATCH] mtd: ubi: actually do work in wear leveling code The actual work in the wear leveling code is done in a separate thread. Since we do not have threading so far we did not do any of the queued work. Change this by calling the worker function synchronously. With this barebox now can write a fastmap on a freshly ubiformated device. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/mtd/ubi/wl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 4535f2d..f24c219 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -183,7 +183,6 @@ static void wl_entry_destroy(struct ubi_device *ubi, struct ubi_wl_entry *e) kfree(e); } -#ifndef CONFIG_MTD_UBI_FASTMAP /** * do_work - do one pending work. * @ubi: UBI device description object @@ -221,7 +220,6 @@ static int do_work(struct ubi_device *ubi) return err; } -#endif /** * in_wl_tree - check if wear-leveling entry is present in a WL RB-tree. @@ -523,8 +521,9 @@ static void __schedule_ubi_work(struct ubi_device *ubi, struct ubi_work *wrk) list_add_tail(&wrk->list, &ubi->works); ubi_assert(ubi->works_count >= 0); ubi->works_count += 1; - if (ubi->thread_enabled && !ubi_dbg_is_bgt_disabled(ubi)) - wake_up_process(ubi->bgt_thread); + + /* No threading in barebox, so do work synchronously */ + do_work(ubi); } /** -- 2.8.1 -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: ubi: Fix endless loop when moving PEB 2016-06-24 11:20 ` Sascha Hauer @ 2016-06-27 8:22 ` Teresa Remmet 2016-06-28 5:26 ` Sascha Hauer 0 siblings, 1 reply; 4+ messages in thread From: Teresa Remmet @ 2016-06-27 8:22 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox Hello Sascha, Am Freitag, den 24.06.2016, 13:20 +0200 schrieb Sascha Hauer: > Hi Teresa, > > On Wed, Jun 22, 2016 at 11:01:57AM +0200, Teresa Remmet wrote: > > When moving a PEB the leb_write_trylock() function is called. > > As the function never returns 0 UBI will end up in an endless > > loop. > > > > Noticed the issue when fastmap has been enabled and data is beeing copied > > several times to a UBI volume. When UBI tries to move the anchor PEB, > > the issue comes up. > > > > The leb_write_trylock() is now equal to the leb_write_lock(). > > But kept it for easier maintaince in future when syncing with > > kernel. > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de> > > --- > > drivers/mtd/ubi/eba.c | 11 +---------- > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c > > index a7af247..31dbcd2 100644 > > --- a/drivers/mtd/ubi/eba.c > > +++ b/drivers/mtd/ubi/eba.c > > @@ -257,16 +257,7 @@ static int leb_write_trylock(struct ubi_device *ubi, int vol_id, int lnum) > > le = ltree_add_entry(ubi, vol_id, lnum); > > if (IS_ERR(le)) > > return PTR_ERR(le); > > - > > - /* Contention, cancel */ > > - le->users -= 1; > > - ubi_assert(le->users >= 0); > > - if (le->users == 0) { > > - rb_erase(&le->rb, &ubi->ltree); > > - kfree(le); > > - } > > - > > - return 1; > > + return 0; > > } > > This seems to be the missing link to make the following patch work. I > have tried this several times but so far never found this bug. Could you > give it a try? With this patch it should be now possible to ubiformat a > device with an image and to generate a fastmap when the device is > detached first. I tested it, and it is working. Ubi now successfully writes an anchor PEB. Teresa > > Sascha > > ------------------------8<--------------------------------- > > From 94cea61ade69f712383e282d6aa573d3a12f11d0 Mon Sep 17 00:00:00 2001 > From: Sascha Hauer <s.hauer@pengutronix.de> > Date: Fri, 24 Jun 2016 11:38:24 +0200 > Subject: [PATCH] mtd: ubi: actually do work in wear leveling code > > The actual work in the wear leveling code is done in a separate thread. > Since we do not have threading so far we did not do any of the queued > work. Change this by calling the worker function synchronously. > > With this barebox now can write a fastmap on a freshly ubiformated > device. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/mtd/ubi/wl.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > index 4535f2d..f24c219 100644 > --- a/drivers/mtd/ubi/wl.c > +++ b/drivers/mtd/ubi/wl.c > @@ -183,7 +183,6 @@ static void wl_entry_destroy(struct ubi_device *ubi, struct ubi_wl_entry *e) > kfree(e); > } > > -#ifndef CONFIG_MTD_UBI_FASTMAP > /** > * do_work - do one pending work. > * @ubi: UBI device description object > @@ -221,7 +220,6 @@ static int do_work(struct ubi_device *ubi) > > return err; > } > -#endif > > /** > * in_wl_tree - check if wear-leveling entry is present in a WL RB-tree. > @@ -523,8 +521,9 @@ static void __schedule_ubi_work(struct ubi_device *ubi, struct ubi_work *wrk) > list_add_tail(&wrk->list, &ubi->works); > ubi_assert(ubi->works_count >= 0); > ubi->works_count += 1; > - if (ubi->thread_enabled && !ubi_dbg_is_bgt_disabled(ubi)) > - wake_up_process(ubi->bgt_thread); > + > + /* No threading in barebox, so do work synchronously */ > + do_work(ubi); > } > > /** > -- > 2.8.1 > > > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: ubi: Fix endless loop when moving PEB 2016-06-27 8:22 ` Teresa Remmet @ 2016-06-28 5:26 ` Sascha Hauer 0 siblings, 0 replies; 4+ messages in thread From: Sascha Hauer @ 2016-06-28 5:26 UTC (permalink / raw) To: Teresa Remmet; +Cc: barebox On Mon, Jun 27, 2016 at 10:22:38AM +0200, Teresa Remmet wrote: > Hello Sascha, > > Am Freitag, den 24.06.2016, 13:20 +0200 schrieb Sascha Hauer: > > Hi Teresa, > > > > On Wed, Jun 22, 2016 at 11:01:57AM +0200, Teresa Remmet wrote: > > > When moving a PEB the leb_write_trylock() function is called. > > > As the function never returns 0 UBI will end up in an endless > > > loop. > > > > > > Noticed the issue when fastmap has been enabled and data is beeing copied > > > several times to a UBI volume. When UBI tries to move the anchor PEB, > > > the issue comes up. > > > > > > The leb_write_trylock() is now equal to the leb_write_lock(). > > > But kept it for easier maintaince in future when syncing with > > > kernel. > > > > > > Signed-off-by: Teresa Remmet <t.remmet@phytec.de> > > > --- > > > drivers/mtd/ubi/eba.c | 11 +---------- > > > 1 file changed, 1 insertion(+), 10 deletions(-) > > > > > > diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c > > > index a7af247..31dbcd2 100644 > > > --- a/drivers/mtd/ubi/eba.c > > > +++ b/drivers/mtd/ubi/eba.c > > > @@ -257,16 +257,7 @@ static int leb_write_trylock(struct ubi_device *ubi, int vol_id, int lnum) > > > le = ltree_add_entry(ubi, vol_id, lnum); > > > if (IS_ERR(le)) > > > return PTR_ERR(le); > > > - > > > - /* Contention, cancel */ > > > - le->users -= 1; > > > - ubi_assert(le->users >= 0); > > > - if (le->users == 0) { > > > - rb_erase(&le->rb, &ubi->ltree); > > > - kfree(le); > > > - } > > > - > > > - return 1; > > > + return 0; > > > } > > > > This seems to be the missing link to make the following patch work. I > > have tried this several times but so far never found this bug. Could you > > give it a try? With this patch it should be now possible to ubiformat a > > device with an image and to generate a fastmap when the device is > > detached first. > > I tested it, and it is working. Ubi now successfully writes an anchor > PEB. Thanks for testing. Applied both patches. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-28 5:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-22 9:01 [PATCH] mtd: ubi: Fix endless loop when moving PEB Teresa Remmet 2016-06-24 11:20 ` Sascha Hauer 2016-06-27 8:22 ` Teresa Remmet 2016-06-28 5:26 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox