On Fri, Jul 02, 2021 at 01:09:05PM +0200, Martin Pieuchot wrote:
> On 01/07/21(Thu) 13:53, Anindya Mukherjee wrote:
> > Hi,
> >
> > I noticed that if I leave the system running for more than about a month,
> > some
> > of the counters in the uvm view of systat(1) overflow and become negative.
> > This
> > is because the members of struct uvmexp in sys/uvm/uvmexp.h are ints. The
> > kernel's internal counters are of course uint64_t so they don't overflow. It
> > only happens during the uvm_sysctl(9) call which casts the numbers to
> > integers.
> > The function is uvmexp_read.
> >
> > In the attached diff I took the path of least resistance and promoted some
> > of
> > the counters to unsigned int. Ideally I would have liked to use int64_t or
> > even
> > uint64_t, but I hit an issue in some of the architecture dependent code. An
> > example is:
> > /usr/src/sys/arch/alpha/alpha/trap.c:536 atomic_add_int(&uvmexp.syscalls,
> > 1);
> > In other places the ++ operator is used to increment the counters and the
> > 64 bit
> > types can be used.
> >
> > I am not completely sure this is the best way to proceed, but even if this
> > diff
> > is horrifying, I'd appreciate some feedback and advice, thanks!
>
> I wonder if we shouldn't use uint64_t for those and embrace the ABI
> break, that would at least simplify the kernel side and remove a
> truncation.
>
> Do you have an idea of the different consumers of the "struct uvmexp"
> apart from systat(1)? What is the impact in userland & ports of such
> change?
I know that golang has its definition of uvmexp and so if you change the
ABI then you would break at least that. struct uvmexp is used more often
than we would like.
> > 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 28 Jun 2021 05:24:56 -0000
> > @@ -329,7 +329,7 @@ uvmexp_read(struct uvmexp *uexp)
> > counters_read(uvmexp_counters, counters, exp_ncounters);
> >
> > /* stat counters */
> > - uexp->faults = (int)counters[faults];
> > + uexp->faults = (unsigned int)counters[faults];
> > uexp->pageins = (int)counters[pageins];
> >
> > /* fault subcounters */
> > @@ -379,10 +379,10 @@ uvmexp_print(int (*pr)(const char *, ...
> > (*pr)(" freemin=%d, free-target=%d, inactive-target=%d, "
> > "wired-max=%d\n", uexp.freemin, uexp.freetarg, uexp.inactarg,
> > uexp.wiredmax);
> > - (*pr)(" faults=%d, traps=%d, intrs=%d, ctxswitch=%d fpuswitch=%d\n",
> > + (*pr)(" faults=%u, traps=%u, intrs=%u, ctxswitch=%u fpuswitch=%d\n",
> > uexp.faults, uexp.traps, uexp.intrs, uexp.swtch,
> > uexp.fpswtch);
> > - (*pr)(" softint=%d, syscalls=%d, kmapent=%d\n",
> > + (*pr)(" softint=%u, syscalls=%u, kmapent=%d\n",
> > uexp.softs, uexp.syscalls, uexp.kmapent);
> >
> > (*pr)(" fault counts:\n");
> > Index: sys/uvm/uvmexp.h
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvmexp.h,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 uvmexp.h
> > --- sys/uvm/uvmexp.h 4 Mar 2021 09:00:03 -0000 1.9
> > +++ sys/uvm/uvmexp.h 28 Jun 2021 05:24:56 -0000
> > @@ -90,12 +90,12 @@ struct uvmexp {
> > int unused06; /* formerly nfreeanon */
> >
> > /* stat counters */
> > - int faults; /* page fault count */
> > - int traps; /* trap count */
> > - int intrs; /* interrupt count */
> > - int swtch; /* context switch count */
> > - int softs; /* software interrupt count */
> > - int syscalls; /* system calls */
> > + unsigned int faults; /* page fault count */
> > + unsigned int traps; /* trap count */
> > + unsigned int intrs; /* interrupt count */
> > + unsigned int swtch; /* context switch count */
> > + unsigned int softs; /* software interrupt count */
> > + unsigned int syscalls; /* system calls */
> > int pageins; /* pagein operation count */
> > /* pageouts are in pdpageouts below */
> > int unused07; /* formerly obsolete_swapins */
> > Index: usr.bin/systat/uvm.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/systat/uvm.c,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 uvm.c
> > --- usr.bin/systat/uvm.c 28 Jun 2019 13:35:04 -0000 1.5
> > +++ usr.bin/systat/uvm.c 28 Jun 2021 05:24:57 -0000
> > @@ -37,22 +37,23 @@ void print_uvm(void);
> > int read_uvm(void);
> > int select_uvm(void);
> >
> > -void print_uvmexp_field(field_def *, field_def *, int *, int *, const char
> > *);
> > +void print_uvmexp_field(field_def *, field_def *, unsigned int *,
> > + unsigned int *, const char *);
> > void print_uvmexp_line(int);
> >
> > struct uvmexp uvmexp;
> > struct uvmexp last_uvmexp;
> >
> > struct uvmline {
> > - int *v1;
> > - int *ov1;
> > - char *n1;
> > - int *v2;
> > - int *ov2;
> > - char *n2;
> > - int *v3;
> > - int *ov3;
> > - char *n3;
> > + unsigned int *v1;
> > + unsigned int *ov1;
> > + char *n1;
> > + unsigned int *v2;
> > + unsigned int *ov2;
> > + char *n2;
> > + unsigned int *v3;
> > + unsigned int *ov3;
> > + char *n3;
> > };
> >
> > struct uvmline uvmline[] = {
> > @@ -214,8 +215,8 @@ read_uvm(void)
> > }
> >
> > void
> > -print_uvmexp_field(field_def *fvalue, field_def *fname, int *new, int *old,
> > - const char *name)
> > +print_uvmexp_field(field_def *fvalue, field_def *fname, unsigned int *new,
> > + unsigned int *old, const char *name)
> > {
> > char *uppername;
> > size_t len, i;
> >
>
--
:wq Claudio
