[U-Boot] UBI: regression since "mtd: ubi: Fix worker handling"
Hi Richard, Heiko
Since f82290afc847 ("mtd: ubi: Fix worker handling"), when booting from NAND, on a fresh NAND just after being flashed (and only in this case), we got the following log:
ubi0: default fastmap pool size: 200 ubi0: default fastmap WL pool size: 100 ubi0: attaching mtd2 ubi0: scanning is finished ubi0 error: ubi_update_fastmap: could not find any anchor PEB ubi0 error: ubi_update_fastmap: could not find any anchor PEB ubi0 error: ubi_wl_get_peb: Unable to get a free PEB from user WL pool ubi0 error: autoresize: cannot auto-resize volume 1 UBI error: cannot attach mtd2UBI error: cannot initialize UBI, error -28UBI init error 28
After analysis, in ubi_wl_init(), when performing schedule_erase(), thread_enabled flag is not yet set to 1, which forbids ubi_do_worker() to execute pending works.
This has to effect to not populate ubi->free with free physical eraseblocks.
As temporary workaround i did the following update on my side which fix this issue, but i don't consider it neither as a clean patch nor certain that it is enough :
@@ -1560,15 +1560,17 @@ int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai) goto out_free;
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e; + ubi->thread_enabled = 1; if (schedule_erase(ubi, e, aeb->vol_id, aeb->lnum, 0)) { + ubi->thread_enabled = 0; wl_entry_destroy(ubi, e); goto out_free; } + ubi->thread_enabled = 0; found_pebs++; }
list_for_each_entry(aeb, &ai->free, u.list) {
Regards
Patrice
Patrice,
Am Montag, 21. Mai 2018, 15:38:57 CEST schrieb Patrice CHOTARD:
Hi Richard, Heiko
Since f82290afc847 ("mtd: ubi: Fix worker handling"), when booting from NAND, on a fresh NAND just after being flashed (and only in this case), we got the following log:
ubi0: default fastmap pool size: 200 ubi0: default fastmap WL pool size: 100 ubi0: attaching mtd2 ubi0: scanning is finished ubi0 error: ubi_update_fastmap: could not find any anchor PEB ubi0 error: ubi_update_fastmap: could not find any anchor PEB ubi0 error: ubi_wl_get_peb: Unable to get a free PEB from user WL pool ubi0 error: autoresize: cannot auto-resize volume 1 UBI error: cannot attach mtd2UBI error: cannot initialize UBI, error -28UBI init error 28
After analysis, in ubi_wl_init(), when performing schedule_erase(), thread_enabled flag is not yet set to 1, which forbids ubi_do_worker() to execute pending works.
This has to effect to not populate ubi->free with free physical eraseblocks.
As temporary workaround i did the following update on my side which fix this issue, but i don't consider it neither as a clean patch nor certain that it is enough :
@@ -1560,15 +1560,17 @@ int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai) goto out_free;
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e;
ubi->thread_enabled = 1;
This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
Thanks, //richard
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e;
ubi->thread_enabled = 1;This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb()
If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics.
Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause joerg.krause@embedded.rocks Signed-off-by: Richard Weinberger <richard@nod.at
Heiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Thanks, //richard
[0] http://lists.infradead.org/pipermail/linux-mtd/2018-May/080965.html
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e;
ubi->thread_enabled = 1;This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause <joerg.krause@embedded.rocks> Signed-off-by: Richard Weinberger <richard@nod.atHeiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
bye, Heiko
Thanks, //richard
[0] http://lists.infradead.org/pipermail/linux-mtd/2018-May/080965.html
Am Dienstag, 22. Mai 2018, 08:30:45 CEST schrieb Heiko Schocher:
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e;
ubi->thread_enabled = 1;This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause <joerg.krause@embedded.rocks> Signed-off-by: Richard Weinberger <richard@nod.atHeiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Ok. :-)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
As I said, commit 1cb8f9776c7dcadc57885c6653943511d282633b should do the trick. Patrice, can you please give it a try?
Thanks, //richard
Hi
On 05/22/2018 08:37 AM, Richard Weinberger wrote:
Am Dienstag, 22. Mai 2018, 08:30:45 CEST schrieb Heiko Schocher:
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e;
ubi->thread_enabled = 1;This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause <joerg.krause@embedded.rocks> Signed-off-by: Richard Weinberger <richard@nod.atHeiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Ok. :-)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
As I said, commit 1cb8f9776c7dcadc57885c6653943511d282633b should do the trick. Patrice, can you please give it a try?
Yes sure,
Thanks
Patrice
Thanks, //richard
Hi Richard, Heiko
On 05/22/2018 08:51 AM, Patrice CHOTARD wrote:
Hi
On 05/22/2018 08:37 AM, Richard Weinberger wrote:
Am Dienstag, 22. Mai 2018, 08:30:45 CEST schrieb Heiko Schocher:
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
e->pnum = aeb->pnum; e->ec = aeb->ec; ubi->lookuptbl[e->pnum] = e; + ubi->thread_enabled = 1;
This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause joerg.krause@embedded.rocks Signed-off-by: Richard Weinberger <richard@nod.at
Heiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Ok. :-)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
As I said, commit 1cb8f9776c7dcadc57885c6653943511d282633b should do the trick. Patrice, can you please give it a try?
I confirm this patch fixes the issue.
Thanks for your reactivity ;-)
Patrice
Yes sure,
Thanks
Patrice
Thanks, //richard
Hello Patrice,
Am 22.05.2018 um 09:21 schrieb Patrice CHOTARD:
Hi Richard, Heiko
On 05/22/2018 08:51 AM, Patrice CHOTARD wrote:
Hi
On 05/22/2018 08:37 AM, Richard Weinberger wrote:
Am Dienstag, 22. Mai 2018, 08:30:45 CEST schrieb Heiko Schocher:
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger:
> e->pnum = aeb->pnum; > e->ec = aeb->ec; > ubi->lookuptbl[e->pnum] = e; > + ubi->thread_enabled = 1;
This is not correct. At this point the UBI thread is not ready. I know, I know, U-Boot has no threads but some data structures might not be ready.
Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause joerg.krause@embedded.rocks Signed-off-by: Richard Weinberger <richard@nod.at
Heiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Ok. :-)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
As I said, commit 1cb8f9776c7dcadc57885c6653943511d282633b should do the trick. Patrice, can you please give it a try?
I confirm this patch fixes the issue.
Thanks for your reactivity ;-)
Thanks for testing! Could you may send a formal patch to the U-Boot ML ?
Thanks!
bye, Heiko
Patrice
Yes sure,
Thanks
Patrice
Thanks, //richard
On 05/22/2018 09:29 AM, Heiko Schocher wrote:
Hello Patrice,
Am 22.05.2018 um 09:21 schrieb Patrice CHOTARD:
Hi Richard, Heiko
On 05/22/2018 08:51 AM, Patrice CHOTARD wrote:
Hi
On 05/22/2018 08:37 AM, Richard Weinberger wrote:
Am Dienstag, 22. Mai 2018, 08:30:45 CEST schrieb Heiko Schocher:
Hello Richard,
Am 21.05.2018 um 21:31 schrieb Richard Weinberger:
Patrice,
Am Montag, 21. Mai 2018, 16:07:41 CEST schrieb Richard Weinberger: >> e->pnum = aeb->pnum; >> e->ec = aeb->ec; >> ubi->lookuptbl[e->pnum] = e; >> + ubi->thread_enabled = 1; > > This is not correct. At this point the UBI thread is not ready. > I know, I know, U-Boot has no threads but some data structures might > not be ready. > > Let me think how to work around this properly.
The root cause seems to be that U-Boot misses this fix from Linux:
commit 1cb8f9776c7dcadc57885c6653943511d282633b Author: Richard Weinberger richard@nod.at Date: Tue Aug 11 23:27:44 2015 +0200
ubi: fastmap: Implement produce_free_peb() If fastmap requests a free PEB for a pool and UBI is busy with erasing PEBs we need to offer a function to wait for one. We can reuse produce_free_peb() from the non-fastmap WL code but with different locking semantics. Cc: stable@vger.kernel.org # 4.1.x- Reported-and-tested-by: Jörg Krause joerg.krause@embedded.rocks Signed-off-by: Richard Weinberger <richard@nod.at
Heiko, I'm currently working on Fastmap in Linux[0]. I suggest to re-syncing with Linux if my changes go upstream, I can ping you them. :-) Fastmap gained many improvements since the last sync.
Yes, but I am currently under load, and doing a rebase is a bigger task I fear (I speculate currently to automate this task with tbot, let me think a little bit about this, if this would work, U-Boot UBI would autaomtically follow linux tree, and I immediately would see merge errors ...)
Ok. :-)
Also before a rebase can go mainline, we need deep testing. So no fast chance here for me ... sorry. If we can find commits, which fix this regression, it may makes sense to apply them before such a rebase ... ?
As I said, commit 1cb8f9776c7dcadc57885c6653943511d282633b should do the trick. Patrice, can you please give it a try?
I confirm this patch fixes the issue.
Thanks for your reactivity ;-)
Thanks for testing! Could you may send a formal patch to the U-Boot ML ?
Ok no problem
Patrice
Thanks!
bye, Heiko
Patrice
Yes sure,
Thanks
Patrice
Thanks, //richard
participants (3)
-
Heiko Schocher -
Patrice CHOTARD -
Richard Weinberger