Am 21.04.2016 um 14:14 schrieb Boris Brezillon:
No idea, if the correct fix not would be to move this erase_worker call after the attach phase ended, as Richard suggested, or if this fix is also valid ...
I discussed that with Richard, and I think moving the ->free_count assignment before iterating over the ->erase list is a good solution.
Ah! Ok, than its fine for me too.
I know the Linux code is assuming that the UBI thread is not launched yet when we call ubi_wl_init(), but to me it seems a bit risky to rely on this assumption (what if we do the UBI thread creation a bit earlier for some reason?). And, of course, as you explained, uboot does not know anything about threads, so all UBI works are executed synchronously, which makes this implementation buggy in uboot.
Hmm... is it also a valid fix for linux then?
Well, it's not required, but it's making the code more future proof IMO. So again, I'll let Richard decide on this aspect.
As discussed with Boris, I'm not a huge fan of the said patch but I understand the need for it. Please send it to linux-mtd I'll apply it.
That said, the root cause of the whole issue is that due to the single thread nature of u-boot UBI work is directly executed at schedule time. For u-boot this works more or less. But UBI allows work being scheduled when the background thread is disabled/paused or not spawned. The free_count patch papers exactly over one of these cases. Let's hope that there are not other (more nasty) cases where u-boot and Linux UBI behave differently. Think of places where work is scheduled but the caller blocked the worker because the work has to be done later. Fastmap is one of these use cases but AFAIK it won't matter as no CPU scheduler is involved and will interrupt Fastmap.
Boris and I worked the last months on a bigger UBI project where we also had to port our UBI changes to u-boot. Now I'm not so sure anymore whether it is a good idea to copy&paste UBI from Linux to u-boot. We faced a lot of issues due to the single thread model. I changed the work model in UBI to make locking less complicated. It turned out that on u-boot it made things more complicated. At least Boris had a lot of "fun". ;-)
In the long run I suggest removing the whole Linux UBI implementation from u-boot and add a small (read only!) implementation which can also read UBIFS. Reading UBIFS is not a big deal. Also journal reply can be done in-memory. Beside of code complexity it will also reduce u-boot's .text size. Thomas' SPL patches are a very good start. I'd also offer my help.
Thanks, //richard