Re: svn commit: r317231 - head/usr.bin/systat
On Fri, 21 Apr 2017, Bruce Evans wrote:
On Thu, 20 Apr 2017, Jung-uk Kim wrote:
Log:
Fix systat(1) regression. It was broken by r317061.
It is more broken than before. Now it fails when the kernel is older
than the utility instead of vice versa. When it fails, the failures
are more serious than before. systat -v actually does complete
checking for errors (some are only errors in itself), but mishandles
them. It reports the errors on the status line and continues. The
error message overwrites the previous one. Continuing is more broken
than before:
The follow patch reduces the ABI breakage as much as possible. On a
lightly loaded system, the maximum value in any counter used by top,
vmstat or systat -v is growing at a rate of 1M per hour, so the 32-bit
variables in the application are enough for about 166 days.
I only applied the fix to some counter sysctls with broken ABIs, but
wrote it for general use. It doesn't handle SYSCTL_IN(), and assumes
unsigned counters.
X Index: subr_counter.c
X ===
X --- subr_counter.c(revision 317243)
X +++ subr_counter.c(working copy)
X @@ -74,6 +75,95 @@
X uma_zfree(pcpu_zone_64, c);
X }
X
X +/* Output a signed integer with the caller's size as well as possible. */
X +__unused
X +static int
X +sysctl_out_i(struct sysctl_req *req, intmax_t val)
X +{
X + intmax_t valtrunc;
X + int64_t val64;
X + int32_t val32;
X + int16_t val16;
X + int8_t val8;
X + int error;
X +
X + switch (req->oldlen) {
X + case 1:
X + valtrunc = val8 = val;
X + error = SYSCTL_OUT(req, , 1);
X + break;
X + case 2:
X + valtrunc = val16 = val;
X + error = SYSCTL_OUT(req, , 2);
X + break;
X + case 4:
X + valtrunc = val32 = val;
X + error = SYSCTL_OUT(req, , 4);
X + break;
X + case 8:
X + valtrunc = val64 = val;
X + error = SYSCTL_OUT(req, , 8);
X + break;
X + default:
X + valtrunc = val;
X + error = SYSCTL_OUT(req, , sizeof(val));
X + break;
X + }
X + if (valtrunc != val && error == 0)
X + error = EOVERFLOW;
X + return (error);
X +}
X +
X +uintmax_t sysoui_max;
X +
X +/* Output an unsigned integer with the caller's size as well as possible. */
X +static int
X +sysctl_out_ui(struct sysctl_req *req, uintmax_t val)
X +{
X + uintmax_t valtrunc;
X + uint64_t val64;
X + uint32_t val32;
X + uint16_t val16;
X + uint8_t val8;
X + int error;
X +
X + if (sysoui_max < val) {
X + sysoui_max = val;
X + if ((val & 0x) == 0)
X + printf("new sysoui_max %#jx\n", val);
X + }
X + val64 = val32 = val16 = val8 = 0;
X + switch (req->oldlen) {
X + case 1:
X + valtrunc = val8 = val;
X + error = SYSCTL_OUT(req, , 1);
X + break;
X + case 2:
X + valtrunc = val16 = val;
X + error = SYSCTL_OUT(req, , 2);
X + break;
X + case 4:
X + valtrunc = val32 = val;
X + error = SYSCTL_OUT(req, , 4);
X + break;
X + case 8:
X + valtrunc = val64 = val;
X + error = SYSCTL_OUT(req, , 8);
X + break;
X + default:
X + valtrunc = val;
X + error = SYSCTL_OUT(req, , sizeof(val));
X + break;
X + }
X + if (valtrunc != val && error == 0) {
X + printf(
X + "val %#jx, valtrunc %#jx, val64 %#jx, val32 %#x, val16 %#x, val8 %#x\n",
X + val, valtrunc, val64, val32, val16, val8);
X + error = EOVERFLOW;
X + }
X + return (error);
X +}
X +
X int
X sysctl_handle_counter_u64(SYSCTL_HANDLER_ARGS)
X {
X @@ -82,7 +172,7 @@
X
X out = counter_u64_fetch(*(counter_u64_t *)arg1);
X
X - error = SYSCTL_OUT(req, , sizeof(uint64_t));
X + error = sysctl_out_ui(req, out);
X
X if (error || !req->newptr)
X return (error);
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r317231 - head/usr.bin/systat
On Thu, 20 Apr 2017, Jung-uk Kim wrote:
Log:
Fix systat(1) regression. It was broken by r317061.
It is more broken than before. Now it fails when the kernel is older
than the utility instead of vice versa. When it fails, the failures
are more serious than before. systat -v actually does complete
checking for errors (some are only errors in itself), but mishandles
them. It reports the errors on the status line and continues. The
error message overwrites the previous one. Continuing is more broken
than before:
Previously, reading 64-bit values into 32-bit variables truncated them,
just like a correct fix would do except for the error messages. Large
values often never occur, and used to be truncated in the kernel, so
noting would be lost by truncating. The support for 64-bit types would
just remain intentionally unfinished instead of wrong. systat has a
lot of modes and fields in which it only reports delta values. Then
truncation doesn't matter unless it occurs more often than the refresh
interval.
Now, reading 32-bit values into 64-bit variabeles leaves garbage in
the top bits. I think the garbage is initially 0 and remains 0, since
the variables are statically initializated and short reads by sysctl()
don't change the top bits. But delta-values are broken if values that
need 64-bit types actually occur. E.g., after increasing a 32-bit
value of 0x with overflow to 1, 32-bit subtraction gave the
correct delta of 2, but 64-bit subtraction gives
1 - (2**32 - 1) = 2 - 2**32 = 0x0002.
Modified: head/usr.bin/systat/vmstat.c
==
--- head/usr.bin/systat/vmstat.cThu Apr 20 21:48:54 2017
(r317230)
+++ head/usr.bin/systat/vmstat.cThu Apr 20 22:30:39 2017
(r317231)
@@ -70,35 +70,35 @@ static const char sccsid[] = "@(#)vmstat
static struct Info {
longtime[CPUSTATES];
- u_int v_swtch; /* context switches */
- u_int v_trap; /* calls to trap */
- u_int v_syscall;/* calls to syscall() */
- u_int v_intr; /* device interrupts */
- u_int v_soft; /* software interrupts */
+ uint64_t v_swtch; /* context switches */
+ uint64_t v_trap;/* calls to trap */
+ uint64_t v_syscall; /* calls to syscall() */
+ uint64_t v_intr;/* device interrupts */
+ uint64_t v_soft;/* software interrupts */
systat -v may also have problems printing large values. It mostly
uses PUTRATE(), and I think that handles 64-bit types not very badly,
though compilers tend to have bugs converting uint64_t to floating
point and systat -v uses float since it doesn't need much precision.
PUTRATE() on a uint64_t calculates the delta in that type and then
scales in double precision and then prints in single precision. The
overflowed value 0x0002 gives a huge rate, say only 1e17
= 100P (P = peta) after scaling down the value from 1.8e19 = 18E (E =
exa). putfloat() only supports up to small numbers of mega and
prints larger values as '*'s.
systat -v thus doesn't actually support 64-bit values. 32-bit values
only go up to 4G or 4294M. With no scaling, 4294M stays large, but
some fields are wide enough to print it so it is not printed as '*'s.
64-bit values go up to 18E.
PUTRATE() already had to be careful with types to get suitable overflow
when calculating the deltas. It is careful enough. It assigns to
unsigned integer fields. This gives modular arithmetic. Expressions
like (new.fld - old.fld) / (float)rate would give sign extension/overflow
bugs depending on the type of fld in a MD way.
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r317231 - head/usr.bin/systat
Thanks, Jung-uk!
On Thu, Apr 20, 2017 at 10:30:39PM +, Jung-uk Kim wrote:
J> Author: jkim
J> Date: Thu Apr 20 22:30:39 2017
J> New Revision: 317231
J> URL: https://svnweb.freebsd.org/changeset/base/317231
J>
J> Log:
J> Fix systat(1) regression. It was broken by r317061.
J>
J> Modified:
J> head/usr.bin/systat/vmstat.c
J>
J> Modified: head/usr.bin/systat/vmstat.c
J>
==
J> --- head/usr.bin/systat/vmstat.c Thu Apr 20 21:48:54 2017
(r317230)
J> +++ head/usr.bin/systat/vmstat.c Thu Apr 20 22:30:39 2017
(r317231)
J> @@ -70,35 +70,35 @@ static const char sccsid[] = "@(#)vmstat
J>
J> static struct Info {
J> longtime[CPUSTATES];
J> -u_int v_swtch; /* context switches */
J> -u_int v_trap; /* calls to trap */
J> -u_int v_syscall;/* calls to syscall() */
J> -u_int v_intr; /* device interrupts */
J> -u_int v_soft; /* software interrupts */
J> +uint64_t v_swtch; /* context switches */
J> +uint64_t v_trap;/* calls to trap */
J> +uint64_t v_syscall; /* calls to syscall() */
J> +uint64_t v_intr;/* device interrupts */
J> +uint64_t v_soft;/* software interrupts */
J> /*
J> * Virtual memory activity.
J> */
J> -u_int v_vm_faults; /* number of address memory faults */
J> -u_int v_io_faults; /* page faults requiring I/O */
J> -u_int v_cow_faults; /* number of copy-on-writes */
J> -u_int v_zfod; /* pages zero filled on demand */
J> -u_int v_ozfod; /* optimized zero fill pages */
J> -u_int v_swapin; /* swap pager pageins */
J> -u_int v_swapout;/* swap pager pageouts */
J> -u_int v_swappgsin; /* swap pager pages paged in */
J> -u_int v_swappgsout; /* swap pager pages paged out */
J> -u_int v_vnodein;/* vnode pager pageins */
J> -u_int v_vnodeout; /* vnode pager pageouts */
J> -u_int v_vnodepgsin; /* vnode_pager pages paged in */
J> -u_int v_vnodepgsout;/* vnode pager pages paged out */
J> -u_int v_intrans;/* intransit blocking page faults */
J> -u_int v_reactivated;/* number of pages reactivated by pagedaemon */
J> -u_int v_pdwakeups; /* number of times daemon has awaken from sleep
*/
J> -u_int v_pdpages;/* number of pages analyzed by daemon */
J> -
J> -u_int v_dfree; /* pages freed by daemon */
J> -u_int v_pfree; /* pages freed by exiting processes */
J> -u_int v_tfree; /* total pages freed */
J> +uint64_t v_vm_faults; /* number of address memory faults */
J> +uint64_t v_io_faults; /* page faults requiring I/O */
J> +uint64_t v_cow_faults; /* number of copy-on-writes */
J> +uint64_t v_zfod;/* pages zero filled on demand */
J> +uint64_t v_ozfod; /* optimized zero fill pages */
J> +uint64_t v_swapin; /* swap pager pageins */
J> +uint64_t v_swapout; /* swap pager pageouts */
J> +uint64_t v_swappgsin; /* swap pager pages paged in */
J> +uint64_t v_swappgsout; /* swap pager pages paged out */
J> +uint64_t v_vnodein; /* vnode pager pageins */
J> +uint64_t v_vnodeout;/* vnode pager pageouts */
J> +uint64_t v_vnodepgsin; /* vnode_pager pages paged in */
J> +uint64_t v_vnodepgsout; /* vnode pager pages paged out */
J> +uint64_t v_intrans; /* intransit blocking page faults */
J> +uint64_t v_reactivated; /* number of pages reactivated by pagedaemon */
J> +uint64_t v_pdwakeups; /* number of times daemon has awaken from sleep
*/
J> +uint64_t v_pdpages; /* number of pages analyzed by daemon */
J> +
J> +uint64_t v_dfree; /* pages freed by daemon */
J> +uint64_t v_pfree; /* pages freed by exiting processes */
J> +uint64_t v_tfree; /* total pages freed */
J> /*
J> * Distribution of page usages.
J> */
J> ___
J> svn-src-...@freebsd.org mailing list
J> https://lists.freebsd.org/mailman/listinfo/svn-src-all
J> To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
--
Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r317231 - head/usr.bin/systat
Author: jkim
Date: Thu Apr 20 22:30:39 2017
New Revision: 317231
URL: https://svnweb.freebsd.org/changeset/base/317231
Log:
Fix systat(1) regression. It was broken by r317061.
Modified:
head/usr.bin/systat/vmstat.c
Modified: head/usr.bin/systat/vmstat.c
==
--- head/usr.bin/systat/vmstat.cThu Apr 20 21:48:54 2017
(r317230)
+++ head/usr.bin/systat/vmstat.cThu Apr 20 22:30:39 2017
(r317231)
@@ -70,35 +70,35 @@ static const char sccsid[] = "@(#)vmstat
static struct Info {
longtime[CPUSTATES];
- u_int v_swtch; /* context switches */
- u_int v_trap; /* calls to trap */
- u_int v_syscall;/* calls to syscall() */
- u_int v_intr; /* device interrupts */
- u_int v_soft; /* software interrupts */
+ uint64_t v_swtch; /* context switches */
+ uint64_t v_trap;/* calls to trap */
+ uint64_t v_syscall; /* calls to syscall() */
+ uint64_t v_intr;/* device interrupts */
+ uint64_t v_soft;/* software interrupts */
/*
* Virtual memory activity.
*/
- u_int v_vm_faults; /* number of address memory faults */
- u_int v_io_faults; /* page faults requiring I/O */
- u_int v_cow_faults; /* number of copy-on-writes */
- u_int v_zfod; /* pages zero filled on demand */
- u_int v_ozfod; /* optimized zero fill pages */
- u_int v_swapin; /* swap pager pageins */
- u_int v_swapout;/* swap pager pageouts */
- u_int v_swappgsin; /* swap pager pages paged in */
- u_int v_swappgsout; /* swap pager pages paged out */
- u_int v_vnodein;/* vnode pager pageins */
- u_int v_vnodeout; /* vnode pager pageouts */
- u_int v_vnodepgsin; /* vnode_pager pages paged in */
- u_int v_vnodepgsout;/* vnode pager pages paged out */
- u_int v_intrans;/* intransit blocking page faults */
- u_int v_reactivated;/* number of pages reactivated by pagedaemon */
- u_int v_pdwakeups; /* number of times daemon has awaken from sleep
*/
- u_int v_pdpages;/* number of pages analyzed by daemon */
-
- u_int v_dfree; /* pages freed by daemon */
- u_int v_pfree; /* pages freed by exiting processes */
- u_int v_tfree; /* total pages freed */
+ uint64_t v_vm_faults; /* number of address memory faults */
+ uint64_t v_io_faults; /* page faults requiring I/O */
+ uint64_t v_cow_faults; /* number of copy-on-writes */
+ uint64_t v_zfod;/* pages zero filled on demand */
+ uint64_t v_ozfod; /* optimized zero fill pages */
+ uint64_t v_swapin; /* swap pager pageins */
+ uint64_t v_swapout; /* swap pager pageouts */
+ uint64_t v_swappgsin; /* swap pager pages paged in */
+ uint64_t v_swappgsout; /* swap pager pages paged out */
+ uint64_t v_vnodein; /* vnode pager pageins */
+ uint64_t v_vnodeout;/* vnode pager pageouts */
+ uint64_t v_vnodepgsin; /* vnode_pager pages paged in */
+ uint64_t v_vnodepgsout; /* vnode pager pages paged out */
+ uint64_t v_intrans; /* intransit blocking page faults */
+ uint64_t v_reactivated; /* number of pages reactivated by pagedaemon */
+ uint64_t v_pdwakeups; /* number of times daemon has awaken from sleep
*/
+ uint64_t v_pdpages; /* number of pages analyzed by daemon */
+
+ uint64_t v_dfree; /* pages freed by daemon */
+ uint64_t v_pfree; /* pages freed by exiting processes */
+ uint64_t v_tfree; /* total pages freed */
/*
* Distribution of page usages.
*/
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
