Date: Sat, 30 Oct 2010 06:58:01 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Pawel Jakub Dawidek <pjd@freebsd.org>, David Xu <davidxu@freebsd.org> Subject: Re: svn commit: r214510 - in head: include lib/libc/gen sys/kern Message-ID: <AANLkTikEVjbLOP15ub=Lh2mDAP88gniM8MO%2B%2Btk5CUZ0@mail.gmail.com> In-Reply-To: <20101030203424.M1007@besplex.bde.org> References: <201010291331.o9TDVAtm027022@svn.freebsd.org> <20101029222159.GA2160@garage.freebsd.pl> <20101030203424.M1007@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--000e0ce02a0a96247c0493d5f6ad
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
On Sat, Oct 30, 2010 at 3:08 AM, Bruce Evans <brde@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:
>>> =A0Add sysctl kern.sched.cpusetsize to export the size of kernel cpuset=
,
>>> =A0also add sysconf() key _SC_CPUSET_SIZE to get sysctl value.
>>>
>>> =A0Submitted by: gcooper
>>
>> [...]
>>>
>>> +#ifdef _SC_CPUSET_SIZE
>>> + =A0 =A0 =A0 case _SC_CPUSET_SIZE:
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 len =3D sizeof(lvalue);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (sysctlbyname("kern.sched.cpusetsize",=
&lvalue, &len,
>>> NULL,
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0) =3D=3D -1)
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (-1);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (lvalue);
>>> +#endif
>>
>> [...]
>>>
>>> +static size_t _kern_cpuset_size =3D 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,
>>> + =A0 =A0&_kern_cpuset_size, 0, "Kernel-level cpuset_t struct size");
>>> +
>
> Just use:
>
> SYSCTL_INT(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
> =A0 =A00, sizeof(cpuset_t), "sizeof(cpuset_t) in the kernel");
>
> the same as for all debugging sizeofs in kern_mib.c. =A0(I also changed t=
he
> style of the message to be more like the ones there. =A0(sysctl -ad | gre=
o
> sizeof on ref9-i386 shows only 1 other inconsistency: the banal descripti=
on
> 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. =A016-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. =A0Also 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. =A0Because 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. =A0(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. =A0It 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. =A0However,
> 16-bit ints wouldn't be. =A0Careful 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
--000e0ce02a0a96247c0493d5f6ad
Content-Type: text/x-patch; charset=US-ASCII;
name="cpuset_t-less_suckage.patch"
Content-Disposition: attachment; filename="cpuset_t-less_suckage.patch"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_gfwk3lu30
SW5kZXg6IHN5cy9rZXJuL3NjaGVkX3VsZS5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9rZXJuL3NjaGVk
X3VsZS5jCShyZXZpc2lvbiAyMTQ1NTQpCisrKyBzeXMva2Vybi9zY2hlZF91bGUuYwkod29ya2lu
ZyBjb3B5KQpAQCAtMjcxMiw4ICsyNzEyLDYgQEAKIAlzYnVmX2RlbGV0ZSh0b3BvKTsKIAlyZXR1
cm4gKGVycik7CiB9Ci0KLXN0YXRpYyBzaXplX3QgX2tlcm5fY3B1c2V0X3NpemUgPSBzaXplb2Yo
Y3B1c2V0X3QpOwogI2VuZGlmCiAKIFNZU0NUTF9OT0RFKF9rZXJuLCBPSURfQVVUTywgc2NoZWQs
IENUTEZMQUdfUlcsIDAsICJTY2hlZHVsZXIiKTsKQEAgLTI3NTEsMTMgKzI3NDksOSBAQAogICAg
IENUTEZMQUdfUkQsIE5VTEwsIDAsIHN5c2N0bF9rZXJuX3NjaGVkX3RvcG9sb2d5X3NwZWMsICJB
IiwgCiAgICAgIlhNTCBkdW1wIG9mIGRldGVjdGVkIENQVSB0b3BvbG9neSIpOwogCi0vKiAKLSAq
IFJldHVybiB0aGUgc2l6ZSBvZiBjcHVzZXRfdCBhdCB0aGUga2VybmVsIGxldmVsCi0gKgotICog
WFhYIChnY29vcGVyKTogcmVwbGFjZSBVTE9ORyB3aXRoIFNJWkUgb25jZSBDVExUWVBFX1NJWkUg
aXMgaW1wbGVtZW50ZWQuCi0gKi8KLVNZU0NUTF9VTE9ORyhfa2Vybl9zY2hlZCwgT0lEX0FVVE8s
IGNwdXNldHNpemUsIENUTEZMQUdfUkQsCi0gICAgJl9rZXJuX2NwdXNldF9zaXplLCAwLCAiS2Vy
bmVsLWxldmVsIGNwdXNldF90IHN0cnVjdCBzaXplIik7CisvKiBSZXR1cm4gdGhlIHNpemUgb2Yg
Y3B1c2V0X3QgYXQgdGhlIGtlcm5lbCBsZXZlbCAqLworU1lTQ1RMX0lOVChfa2Vybl9zY2hlZCwg
T0lEX0FVVE8sIGNwdXNldHNpemUsIENUTEZMQUdfUkQsCisgICAgMCwgc2l6ZW9mKGNwdXNldF90
KSwgInNpemVvZihjcHVzZXRfdCkiKTsKIAogI2VuZGlmCiAK
--000e0ce02a0a96247c0493d5f6ad--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikEVjbLOP15ub=Lh2mDAP88gniM8MO%2B%2Btk5CUZ0>