Hello Richard,
Am 22.04.2016 um 11:34 schrieb 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.
Ok, done.
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.
Can you explain this a little bit?
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". ;-)
Heh...
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.
Hmm.. I think read only is not for all boards an option, as we also
create UBI Volumes and/or write to them in U-Boot ...
Beside of code complexity it will also reduce u-boot's .text size.
Thomas' SPL patches are a very good start.
Yes, I hope to get soon an Ack/Response from Ladislav and/or Enric,
so we can integrate them into mainline.
I'd also offer my help.
Thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
