[U-Boot] [PATCH] mtd, ubi: set free_count to zero before walking through erase list

[Richard Weinberger]

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