On Thu, 8 Nov 2012, Marius Strobl wrote:
On Thu, Nov 08, 2012 at 09:41:29PM +1100, Bruce Evans wrote:
On Thu, 8 Nov 2012, Marius Strobl wrote:
Log:
Make r242655 build on sparc64. While at it, make
vm_{max,min}_kernel_address
vm_offset_t as they should be.
Er, they shouldn't be vm_offset_t.
Modified: head/sys/kern/kern_malloc.c
==============================================================================
--- head/sys/kern/kern_malloc.c Thu Nov 8 04:02:36 2012 (r242746)
+++ head/sys/kern/kern_malloc.c Thu Nov 8 08:10:32 2012 (r242747)
@@ -186,11 +186,13 @@ struct {
*/
static uma_zone_t mt_zone;
-static u_long vm_min_kernel_address = VM_MIN_KERNEL_ADDRESS;
+static vm_offset_t vm_min_kernel_address = VM_MIN_KERNEL_ADDRESS;
SYSCTL_ULONG(_vm, OID_AUTO, min_kernel_address, CTLFLAG_RD,
&vm_min_kernel_address, 0, "Min kernel address");
SYSCTL_ULONG takes a u_long, not a vm_offset_t. A cast in
SYSCTL_ULONG() prevents possible detection of the type mismatch from
this.
This is most broken on i386's with correctly-sized longs (or almost
any arch with correctly-sized longs). There, vm_offset_t is 1
register wide and longs are 2 registers wide.
Eh, FreeBSD/i386 is ILP32, so longs are 32-bit there, as is its
__vm_offset_t.
Yes, it has incorrectly sized longs. It used to have a _LARGE_LONG
option to give 64-bit longs, and I used this to find and fix some
type errors before there were any 64-bit arches (most things compiled
and parts of userland ran).
The bugs could be moved using SYSCTL_QUAD(). Correctly-sized quads
would be 4 registers wide, except quad has come to mean just a bad
way of spelling long long or int64_t.
-static u_long vm_max_kernel_address = VM_MAX_KERNEL_ADDRESS;
+#ifndef __sparc64__
+static vm_offset_t vm_max_kernel_address = VM_MAX_KERNEL_ADDRESS;
+#endif
SYSCTL_ULONG(_vm, OID_AUTO, max_kernel_address, CTLFLAG_RD,
&vm_max_kernel_address, 0, "Max kernel address");
Since the value is a compile-time constant, u_long has a chance of
holding its value even if u_long != vm_offset_t, and the old code is
closer to being correct than first appeared, and the correct fix is
relatively easy -- just use a uintmax_t vatiable and SYSCTL_UINTMAX()
(*). Unfortunately SYSCT_UINTMAX() doesn't exist, and the bogus
SYSCTL_UQUAD() does exist.
Right, SYSCTL_UINTMAX() unfortunately doesn't exist. SYSCTL_UQUAD()
seemed inappropriate as it is 64-bit and we want 32-bit for ILP32
for an exact match. Using uintmax_t with SYSCTL_UQUAD() also just
happens to be of the same width.
From the available combinations, using vm_offset_t with SYSCTL_ULONG()
just seemed to suck the least.
This is backwards, since it u_long that the API supports, so to use the
API it must be assumed that vm_offset_t can be punned to a u_long, not
vice versa although the difference is only logical.
On sparc64, as on most or all 64-bit arches, vm_offset_t is identical to
u_long, so this doesn't even involve type puns. On i386's with
incorrectly-sized longs (ILP32), there is a type pun from vm_offset_t =
u_int to u_long. On i386's with correctly-sized longs (I32L64P32),
there is a type mismatch. In this case, making the variable u_long to
match the API works correctly (when the type error is not detected),
since the top 32 bits are just always zero because the sysctl is r/o
so no one can can change them from their initial zero values. The
reverse dosen't work, since it reads 32 bits beyond the end of the
variable, giving garbage results. The overrun would be a more harmful
for a r/w sysctl.
MD code mostly hard-codes assumptions that vm_offset_t is u_int or u_long
for printing it -- it doesn't laboriously convert vm_offset_t to uintmax_t
as is strictly necessary for printing all MI typedefs. This corresponds
to punning vm_offset_t to u_long here, except the assumptions are much
larger in MI code.
(*) Except a temporary variable is just a style bug in the above and
for this. SYSCTL_UNLONG(), like all integer SYSCTL()'s, has the feature
of supporting either a variable or a constant arg. The above should
use a constant arg and not need a variable. IIRC, the syntax for this is:
SYSCTL_ULONG(_vm, OID_AUTO, max_kernel_address, CTLFLAG_RD,
NULL, VM_MAX_KERNEL_ADDRESS, "Max kernel address");
Actually, for sparc64 VM_MAX_KERNEL_ADDRESS isn't constant (and can't
be).
I didn't notice that when I replied before. The declaration would have
to be ifdefed, as you did but with the correct type for the MI variable.
The MD variable can't have the correct type for this since it needs to
remain vm_offset_t to match other APIs. Also, the ifdefs are simpler
if a variable (with the same name) is used for all cases.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
