Re: uvm_meter: improve periodic execution logic for uvm_loadav()

[Scott Cheloha]

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.

Index: uvm_meter.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_meter.c,v
retrieving revision 1.42
diff -u -p -r1.42 uvm_meter.c
--- uvm_meter.c 28 Dec 2020 14:01:23 -0000      1.42
+++ uvm_meter.c 18 Jun 2023 17:11:09 -0000
@@ -87,8 +87,16 @@ void uvmexp_read(struct uvmexp *);
 void
 uvm_meter(void)
 {
-       if ((gettime() % 5) == 0)
+       static time_t next_uvm_loadav;
+       time_t intvl_count, now;
+
+       now = getuptime();
+       if (now >= next_uvm_loadav) {
+               intvl_count = (now - next_uvm_loadav) / 5 + 1;
+               next_uvm_loadav += 5 * intvl_count;
                uvm_loadav(&averunnable);
+       }
+
        if (proc0.p_slptime > (maxslp / 2))
                wakeup(&proc0);
 }