Date: Fri, 21 Apr 2017 14:15:19 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Jung-uk Kim <jkim@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r317231 - head/usr.bin/systat Message-ID: <20170421131652.P966@besplex.bde.org> In-Reply-To: <201704202230.v3KMUdAm030762@repo.freebsd.org> References: <201704202230.v3KMUdAm030762@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 0xffffffff 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 = 0xffffffff00000002.
> Modified: head/usr.bin/systat/vmstat.c
> ==============================================================================
> --- head/usr.bin/systat/vmstat.c Thu Apr 20 21:48:54 2017 (r317230)
> +++ head/usr.bin/systat/vmstat.c Thu Apr 20 22:30:39 2017 (r317231)
> @@ -70,35 +70,35 @@ static const char sccsid[] = "@(#)vmstat
>
> static struct Info {
> long time[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 0xffffffff00000002 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<unit> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170421131652.P966>