Date: Sat, 30 Oct 2010 21:08:56 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Pawel Jakub Dawidek <pjd@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, David Xu <davidxu@freebsd.org> Subject: Re: svn commit: r214510 - in head: include lib/libc/gen sys/kern Message-ID: <20101030203424.M1007@besplex.bde.org> In-Reply-To: <20101029222159.GA2160@garage.freebsd.pl> References: <201010291331.o9TDVAtm027022@svn.freebsd.org> <20101029222159.GA2160@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 30 Oct 2010, Pawel Jakub Dawidek wrote:
> On Fri, Oct 29, 2010 at 01:31:10PM +0000, David Xu wrote:
>> Log:
>> Add sysctl kern.sched.cpusetsize to export the size of kernel cpuset,
>> also add sysconf() key _SC_CPUSET_SIZE to get sysctl value.
>>
>> Submitted by: gcooper
> [...]
>> +#ifdef _SC_CPUSET_SIZE
>> + case _SC_CPUSET_SIZE:
>> + len = sizeof(lvalue);
>> + if (sysctlbyname("kern.sched.cpusetsize", &lvalue, &len, NULL,
>> + 0) == -1)
>> + return (-1);
>> + return (lvalue);
>> +#endif
> [...]
>> +static size_t _kern_cpuset_size = sizeof(cpuset_t);
No need for this or its bogus type, since it is a small compile-time
constant value.
> [...]
>> +/*
>> + * Return the size of cpuset_t at the kernel level
>> + *
>> + * XXX (gcooper): replace ULONG with SIZE once CTLTYPE_SIZE is implemented.
>> + */
>> +SYSCTL_ULONG(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
>> + &_kern_cpuset_size, 0, "Kernel-level cpuset_t struct size");
>> +
Just use:
SYSCTL_INT(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
0, sizeof(cpuset_t), "sizeof(cpuset_t) in the kernel");
the same as for all debugging sizeofs in kern_mib.c. (I also changed the
style of the message to be more like the ones there. (sysctl -ad | greo
sizeof on ref9-i386 shows only 1 other inconsistency: the banal description
is missing for debug.sizeof.namecache.))
> Because it is used via sysconf(3), I don't think it should be converted
> to CTLTYPE_SIZE at all. I even think it would be safer to make
> _kern_cpuset_size a long (sysconf's lvalue is long) and (just for
> consistency) use SYSCTL_LONG().
>
> Also note, that on i386 long is 32bit and on amd64 long is 64bit, so
> 32bit process running on 64bit system won't be able to read this sysctl.
> Or do we detect 32bit processes on 64bit systems and convert such types
> in the kernel?
Just use int. 16-bit ints are only large enough for 8*32767 CPUs, but
32-bit ints are large enough for 8*2147482647 CPUs, which should be
enough for anyone. Also use 'int value' instead of 'long lvalue' in
sysconf.c.
Using u_long is also bogus. Because the value is returned by sysconf(3),
u_long won't actually work if it is actually needed, because if the
value exceeds LONG_MAX then `return (lvalue)' will blindly overflow.
But the value is very far from exeeding even INT_MAX, so there is no
problem. (Overflow would actually occur earlier, via the type pun of
using sysctl for a u_long variable to read the variable into a plain
long. It is unclear that this type pun is safe even when there is no
overflow.)
The kernel isn't going to be having any data structures larger than
2147482647 bytes any time soon, so 32-bit ints are plenty large enough
for returning the size of any data structure in the kernel. However,
16-bit ints wouldn't be. Careful code might use size_t to avoid
assuming anything about sizeof(int), but with 32-bit ints this mainly
gives bloat when size_t is 64 bits.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101030203424.M1007>