Hi Stefan,
On 2/27/25 13:30, Stefan Roese wrote:
> Hi Jerome,
>
> On 25.02.25 17:34, Jerome Forissier wrote:
>> Make the schedule() call from the CYCLIC framework a uthread scheduling
>> point too. This makes sense since schedule() is called from a lot of
>> places where uthread_schedule() needs to be called.
>>
>> Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
>
> Frankly, at first I was wondering a bit, if and why another framework
> for "multitasking" is needed in U-Boot, additionally to the cyclic
> framework that I introduced a few years ago. Which was greatly enhanced
> by Rasmus over the time. But looking at your "uthread" implementation
> it makes sense to add such a probably more intuitive interface as well.
cyclic is clean and simple and certainly well suited when introducing new
code. But when reworking older code I find it somewhat difficult to use
due to the need to keep a context and pass it everywhere. This can lead
to lots of changes when call stacks are deep.
> In general I'm really happy seeing activity in this "multitaking" area
> in U-Boot. As it brings a lot of new possibilities and, as you've also
> shown in your patchset, may greatly help reducing boot time in the
> USB example. :)
Thank you.
> One question though:
> Do you have some means in your uthread framework, measuring and
> perhaps limiting the time spent in these uthreads? If and how is a
> preemption of a uthread possible? So that it does not consume too
> much time resulting in e.g. things like dropping input chars on
> the prompt? Sorry, I did not thoroughly go through all your code
> to get the internals from there. It would be great if you could
> elaborate a bit on this.
That's a very valid point. The short answer is no, there is no control
over how long a thread keeps the CPU busy. uthread is similar to cyclic
in that respect. That said, I occasionally noted the issue you mentioned
about the console dropping characters, and yes it is annoying. I believe
there is a simple solution though. If we can somehow make sure we're
always scheduling the main thread every other time, we're much less
likely to starve it from the CPU, especially when many threads are
active. That is, assume we have 2 threads in addition to the main thread.
The thread list is main -> thread1 -> thread2 and uthread_schedule() will
iterate in that order. So back to main only after thread1 *and* thread2
have run and called uthread_schedule(). The idea is to schedule in a
different order: main -> thread1 -> main -> thread2 -> main etc.,
effectively giving a higher priority to the main thread (which would be
the console parsing thread).
I feel that introducing preemption would be opening a can of worms...
Because in this case we would likely need locking everywhere. Without
preemption, we still do need locking in theory, it's just that I have
not yet identified critical sections where locks would be mandatory in
the code that I have "parallelized". BTW I believe a uthread lock would
be trivial to implement like so:
struct uthread_lock {
bool locked;
};
void uthread_lock(struct uthread_lock *l)
{
while (l->locked)
uthread_schedule();
l->locked = true;
}
void uthread_unlock(struct uthread_lock *l)
{
l->locked = false;
}
>
> For this patch:
>
> Reviewed-by: Stefan Roese <s...@denx.de>
>
> Thanks,
> Stefan
Thanks,
--
Jerome
>
>> ---
>> common/cyclic.c | 3 +++
>> include/u-boot/schedule.h | 3 +++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/common/cyclic.c b/common/cyclic.c
>> index fad071a39c6..b695f092f52 100644
>> --- a/common/cyclic.c
>> +++ b/common/cyclic.c
>> @@ -16,6 +16,7 @@
>> #include <linux/list.h>
>> #include <asm/global_data.h>
>> #include <u-boot/schedule.h>
>> +#include <uthread.h>
>> DECLARE_GLOBAL_DATA_PTR;
>> @@ -100,6 +101,8 @@ void schedule(void)
>> */
>> if (gd)
>> cyclic_run();
>> +
>> + uthread_schedule();
>> }
>> int cyclic_unregister_all(void)
>> diff --git a/include/u-boot/schedule.h b/include/u-boot/schedule.h
>> index 4fd34c41229..4605971fdcb 100644
>> --- a/include/u-boot/schedule.h
>> +++ b/include/u-boot/schedule.h
>> @@ -3,6 +3,8 @@
>> #ifndef _U_BOOT_SCHEDULE_H
>> #define _U_BOOT_SCHEDULE_H
>> +#include <uthread.h>
>> +
>> #if CONFIG_IS_ENABLED(CYCLIC)
>> /**
>> * schedule() - Schedule all potentially waiting tasks
>> @@ -17,6 +19,7 @@ void schedule(void);
>> static inline void schedule(void)
>> {
>> + uthread_schedule();
>> }
>> #endif
>
> Viele Grüße,
> Stefan Roese
>
