On Sun, Jun 18, 2023 at 12:43:18PM -0500, Scott Cheloha wrote:
> On Sun, Jun 18, 2023 at 12:36:07PM -0500, Scott Cheloha wrote:
> > On Sun, Jun 18, 2023 at 07:32:56PM +0200, Mark Kettenis wrote:
> > > > Date: Sun, 18 Jun 2023 12:27:17 -0500
> > > > From: Scott Cheloha <scottchel...@gmail.com>
> > > > 
> > > > The intent here is to update the load averages every five seconds.
> > > > However:
> > > > 
> > > > 1. Measuring elapsed time with the UTC clock is unreliable because of
> > > >    settimeofday(2).
> > > > 
> > > > 2. "Call uvm_loadav() no more than once every five seconds", is not
> > > >     equivalent to "call uvm_loadav() if the current second is equal
> > > >     to zero, modulo five".
> > > > 
> > > >    Not hard to imagine edge cases where timeouts are delayed and
> > > >    the load averages are not updated.
> > > > 
> > > > So, (1) use the monotonic clock, and (2) keep the next uvm_loadav()
> > > > call time in a static value.
> > > > 
> > > > ok?
> > > 
> > > I really don't see why the calculatin of something vague like the load
> > > average warrants complicating the code like this.
> > 
> > Aren't load averages used to make decisions about thread placement in
> > the scheduler?
> > 
> > Regardless, the code is still wrong.  At minimum you should use
> > getuptime(9).
> 
> Maybe I misunderstood.  Are you suggesting this?
> 
> 
>       now = getuptime();
>       if (now >= next_uvm_loadav) {
>               next_uvm_loadav = now + 5;
>               uvm_loadav(...);
>       }
> 
> The patch I posted preserves the current behavior.  It is equivalent
> to:
> 
>       while (next_uvm_loadav <= now)
>               next_uvm_loadav += 5;
> 
> Changing it to (now + 5) changes the behavior.

To be honest, I think the uvm_meter should be called via timeout(9) and
not be called via the current path using schedcpu().
At some point schedcpu() may be removed and then we need to fix this
proper anyway.
Running the lbolt wakeup inside schedcpu() has the same issue.

-- 
:wq Claudio

Reply via email to