On Tue, Jun 20, 2023 at 11:47:10AM +0200, Claudio Jeker wrote:
> On Mon, Jun 19, 2023 at 04:45:03PM -0500, Scott Cheloha wrote:
>
> [...]
>
> > Index: uvm/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/uvm_meter.c 28 Dec 2020 14:01:23 -0000 1.42
> > +++ uvm/uvm_meter.c 19 Jun 2023 21:35:22 -0000
> > @@ -42,6 +42,7 @@
> > #include <sys/percpu.h>
> > #include <sys/proc.h>
> > #include <sys/sysctl.h>
> > +#include <sys/timeout.h>
> > #include <sys/vmmeter.h>
> > #include <uvm/uvm.h>
> > #include <uvm/uvm_ddb.h>
> > @@ -65,6 +66,9 @@
> > int maxslp = MAXSLP; /* patchable ... */
> > struct loadavg averunnable;
> >
> > +/* Update load averages every five seconds. */
> > +#define UVM_METER_INTVL 5
> > +
> > /*
> > * constants for averages over 1, 5, and 15 minutes when sampling at
> > * 5 second intervals.
> > @@ -78,17 +82,29 @@ static fixpt_t cexp[3] = {
> >
> >
> > static void uvm_loadav(struct loadavg *);
> > +void uvm_meter(void *);
> > void uvm_total(struct vmtotal *);
> > void uvmexp_read(struct uvmexp *);
> >
> > +void
> > +uvm_meter_start(void)
> > +{
> > + static struct timeout to = TIMEOUT_INITIALIZER(uvm_meter, &to);
> > +
> > + uvm_meter(&to);
> > +}
> > +
> > /*
> > * uvm_meter: calculate load average and wake up the swapper (if needed)
> > */
> > void
> > -uvm_meter(void)
> > +uvm_meter(void *arg)
> > {
> > - if ((gettime() % 5) == 0)
> > - uvm_loadav(&averunnable);
> > + struct timeout *to = arg;
> > +
> > + timeout_add_sec(to, UVM_METER_INTVL);
> > +
> > + uvm_loadav(&averunnable);
> > if (proc0.p_slptime > (maxslp / 2))
> > wakeup(&proc0);
> > }
>
> Why add uvm_meter_start() using a static global value and then pass that
> value around. This code could just be:
>
> struct timeout uvm_meter_to = TIMEOUT_INITIALIZER(uvm_meter, NULL);
>
> void
> uvm_meter(void *arg)
> {
> timeout_add_sec(&uvm_meter_to, UVM_METER_INTVL);
> uvm_loadav(&averunnable);
> }
>
> and then just call uvm_meter() once in scheduler_start().
> I don't understand why all extra this indirection is needed it does not
> make the code better..
>
> Apart from that and the fact that the proc0 wakeup and go I'm OK with this
> diff.
I like that better. I'll commit the attached tomorrow unless I hear
otherwise.
Index: share/man/man9/uvm_init.9
===================================================================
RCS file: /cvs/src/share/man/man9/uvm_init.9,v
retrieving revision 1.5
diff -u -p -r1.5 uvm_init.9
--- share/man/man9/uvm_init.9 21 May 2023 05:11:38 -0000 1.5
+++ share/man/man9/uvm_init.9 20 Jun 2023 15:20:59 -0000
@@ -168,7 +168,7 @@ argument is ignored.
.Ft void
.Fn uvm_kernacc "caddr_t addr" "size_t len" "int rw"
.Ft void
-.Fn uvm_meter
+.Fn uvm_meter "void *"
.Ft int
.Fn uvm_sysctl "int *name" "u_int namelen" "void *oldp" "size_t *oldlenp"
"void *newp " "size_t newlen" "struct proc *p"
.Ft int
@@ -212,7 +212,7 @@ access, in the kernel address space.
.Pp
The
.Fn uvm_meter
-function calculates the load average and wakes up the swapper if necessary.
+function periodically recomputes the load average.
.Pp
The
.Fn uvm_sysctl
Index: sys/kern/sched_bsd.c
===================================================================
RCS file: /cvs/src/sys/kern/sched_bsd.c,v
retrieving revision 1.74
diff -u -p -r1.74 sched_bsd.c
--- sys/kern/sched_bsd.c 4 Feb 2023 19:33:03 -0000 1.74
+++ sys/kern/sched_bsd.c 20 Jun 2023 15:20:59 -0000
@@ -234,7 +234,6 @@ schedcpu(void *arg)
}
SCHED_UNLOCK(s);
}
- uvm_meter();
wakeup(&lbolt);
timeout_add_sec(to, 1);
}
@@ -669,6 +668,7 @@ scheduler_start(void)
rrticks_init = hz / 10;
schedcpu(&schedcpu_to);
+ uvm_meter(NULL);
#ifndef SMALL_KERNEL
if (perfpolicy == PERFPOL_AUTO)
Index: sys/uvm/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
--- sys/uvm/uvm_meter.c 28 Dec 2020 14:01:23 -0000 1.42
+++ sys/uvm/uvm_meter.c 20 Jun 2023 15:20:59 -0000
@@ -65,6 +65,9 @@
int maxslp = MAXSLP; /* patchable ... */
struct loadavg averunnable;
+#define UVM_METER_INTVL 5
+struct timeout uvm_meter_to = TIMEOUT_INITIALIZER(uvm_meter, NULL);
+
/*
* constants for averages over 1, 5, and 15 minutes when sampling at
* 5 second intervals.
@@ -82,15 +85,13 @@ void uvm_total(struct vmtotal *);
void uvmexp_read(struct uvmexp *);
/*
- * uvm_meter: calculate load average and wake up the swapper (if needed)
+ * uvm_meter: recompute load averages
*/
void
-uvm_meter(void)
+uvm_meter(void *unused)
{
- if ((gettime() % 5) == 0)
- uvm_loadav(&averunnable);
- if (proc0.p_slptime > (maxslp / 2))
- wakeup(&proc0);
+ timeout_add_sec(&uvm_meter_to, UVM_METER_INTVL);
+ uvm_loadav(&averunnable);
}
/*
Index: sys/uvm/uvm_extern.h
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_extern.h,v
retrieving revision 1.168
diff -u -p -r1.168 uvm_extern.h
--- sys/uvm/uvm_extern.h 30 May 2023 08:30:01 -0000 1.168
+++ sys/uvm/uvm_extern.h 20 Jun 2023 15:20:59 -0000
@@ -414,7 +414,7 @@ void uvmspace_free(struct vmspace *);
struct vmspace *uvmspace_share(struct process *);
int uvm_share(vm_map_t, vaddr_t, vm_prot_t,
vm_map_t, vaddr_t, vsize_t);
-void uvm_meter(void);
+void uvm_meter(void *);
int uvm_sysctl(int *, u_int, void *, size_t *,
void *, size_t, struct proc *);
struct vm_page *uvm_pagealloc(struct uvm_object *,
