svn commit: r214510 - in head: include lib/libc/gen sys/kern

[Garrett Cooper]

On Sat, Oct 30, 2010 at 3:08 AM, Bruce Evans <brde at optusnet.com.au> wrote:
> 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.))

Yeah... it was silly of me to do it that way, in hindsight :(...

>> 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.

That won't really do much good, otherwise it would oppose the POSIX
API definition :/...

> 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.

    Ok.
Thanks!
-Garrett
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpuset_t-less_suckage.patch
Type: text/x-patch
Size: 947 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20101030/1ad63472/cpuset_t-less_suckage.bin