Re: [PATCH v2 08/14] lib: time: hook uthread_schedule() into udelay()

[Yao Zi <ziyao@disroot.org>]

On Fri, Feb 28, 2025 at 03:38:22PM +0200, Ilias Apalodimas wrote:
> Hi Jerome,
> 
> On Tue, 25 Feb 2025 at 18:35, Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
> >
> > Introduce a uthread scheduling loop into udelay() when CONFIG_UTHREAD
> > is enabled. This means that any uthread calling into udelay() may yield
> > to uthread and be scheduled again later.
> >
> > While not strictly necessary since uthread_schedule() is already called
> > by schedule(),
> > tests show that it is desirable to call it in a tight
> > loop instead of calling __usleep(). It gives more opportunities for
> > other threads to make progress and results in better performances.
> 
> Some examples of timing gains would be nice.
> 
> >
> > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> > ---
> >  lib/time.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/time.c b/lib/time.c
> > index d88edafb196..d1a1a66f301 100644
> > --- a/lib/time.c
> > +++ b/lib/time.c
> > @@ -17,6 +17,7 @@
> >  #include <asm/global_data.h>
> >  #include <asm/io.h>
> >  #include <linux/delay.h>
> > +#include <uthread.h>
> >
> >  #ifndef CFG_WD_PERIOD
> >  # define CFG_WD_PERIOD (10 * 1000 * 1000)      /* 10 seconds default */
> > @@ -197,7 +198,14 @@ void udelay(unsigned long usec)
> >         do {
> >                 schedule();
> >                 kv = usec > CFG_WD_PERIOD ? CFG_WD_PERIOD : usec;
> > -               __udelay(kv);
> > +               if (CONFIG_IS_ENABLED(UTHREAD)) {
> > +                       ulong t0 = timer_get_us();
> > +                       while (timer_get_us() < t0 + kv)
> 
> Do we make progress by constantly scheduling new tasks? Perhaps we
> should at least leave the task running for some time?

If I get the point, the UTHREAD is a cooperative framework, which means
a task yields the control flow only when it considers nothing else could
be done. And there's no preemption (at least in this revision). Thus I
don't think it's a problem.

> Thanks
> /Ilias

Best regards,
Yao Zi

> > +                               uthread_schedule();
> > +               } else {
> > +                       __udelay(kv);
> > +               }
> >                 usec -= kv;
> >         } while(usec);
> > +
> >  }
> > --
> > 2.43.0
> >