On Tue, Jun 20, 2023 at 10:25:02AM -0500, Scott Cheloha wrote:
> 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 *,
OK claudio@
--
:wq Claudio
