Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Jun 2011 16:02:12 +0400
From:      Sergey Kandaurov <pluknet@freebsd.org>
To:        David Xu <davidxu@freebsd.org>, Bruce Evans <bde@freebsd.org>
Cc:        Attilio Rao <attilio@freebsd.org>, FreeBSD Current <freebsd-current@freebsd.org>
Subject:   Re: svn commit: r214611 - head/sys/kern
Message-ID:  <BANLkTi=TL4Yz5oR88qqorBvouWm0aYYmiQ@mail.gmail.com>
In-Reply-To: <201011010042.oA10gPeg016326@svn.freebsd.org>
References:  <201011010042.oA10gPeg016326@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 1 November 2010 03:42, David Xu <davidxu@freebsd.org> wrote:
> Author: davidxu
> Date: Mon Nov =A01 00:42:25 2010
> New Revision: 214611
> URL: http://svn.freebsd.org/changeset/base/214611
>
> Log:
> =A0Use integer for size of cpuset, as it won't be bigger than INT_MAX,
> =A0This is requested by bge.
> =A0Also move the sysctl into file kern_cpuset.c, because it should
> =A0always be there, it is independent of thread scheduler.

Hi.
This breaks for me fetching a cpusetsize value with sysconf(3) interface,
as after this change sysconf(3) consumers expect a long return type, while
sysctl kern.sched.cpusetsize has switched from long to int type in kernel.
That makes for me sizeof(cpusize_t) from 8 to incorrect 34359738376.

In particular, kvm_getpcpu(3) uses sysconf(3) to fetch cpusetsize on
live kernel. That gives me a broken result:
kvm_open: kcpusetsize: 8
pcpu[0] =3D 0x801072300
kvm_open: kcpusetsize: 34359738376
pcpu[1] =3D 0xffffffffffffffff
kvm_open: kcpusetsize: 8
pcpu[2] =3D 0x801072600
kvm_open: kcpusetsize: 34359738376
pcpu[3] =3D 0xffffffffffffffff

This small test indicates that that's due to int->long type conversion:
        long lvalue;
        size_t len;

        len =3D sizeof(lvalue);
        if (sysctlbyname("kern.sched.cpusetsize", &lvalue, &len, NULL, 0) <=
 0)
                err(1, "sysctlbyname");
        printf("sysctl: %ld\n", lvalue);
        printf("sysctl: %d -- explicitly casted to (int)\n", (int)lvalue);
        printf("sysconf: %ld\n", sysconf(_SC_CPUSET_SIZE));
        printf("sysconf: %d -- explicitly casted to (int)\n",
(int)sysconf(_SC_CPUSET_SIZE));

That prints:
sysctl: 34359738376
sysctl: 8 -- explicitly casted to (int)
sysconf: 34359738376
sysconf: 8 -- explicitly casted to (int)

The other way to solve this other than reverting is to "fix" all cpusetsize
consumers in userland. Now sysconf() saves long returned value to int:

Index: lib/libkvm/kvm_pcpu.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- lib/libkvm/kvm_pcpu.c       (revision 223073)
+++ lib/libkvm/kvm_pcpu.c       (working copy)
@@ -120,7 +120,7 @@
 void *
 kvm_getpcpu(kvm_t *kd, int cpu)
 {
-       long kcpusetsize;
+       int kcpusetsize;
        ssize_t nbytes;
        uintptr_t readptr;
        char *buf;

So, after applying the above change all is ok:
kvm_open: kcpusetsize: 8
pcpu[0] =3D 0x801072300
kvm_open: kcpusetsize: 8
pcpu[1] =3D 0x801072600
kvm_open: kcpusetsize: 8
pcpu[2] =3D 0x801072900
kvm_open: kcpusetsize: 8
pcpu[3] =3D 0x801072c00


>
> Modified:
> =A0head/sys/kern/kern_cpuset.c
> =A0head/sys/kern/sched_ule.c
>
> Modified: head/sys/kern/kern_cpuset.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/kern/kern_cpuset.c Sun Oct 31 23:04:15 2010 =A0 =A0 =A0 =A0(=
r214610)
> +++ head/sys/kern/kern_cpuset.c Mon Nov =A01 00:42:25 2010 =A0 =A0 =A0 =
=A0(r214611)
> @@ -107,6 +107,10 @@ static struct setlist cpuset_ids;
> =A0static struct unrhdr *cpuset_unr;
> =A0static struct cpuset *cpuset_zero;
>
> +/* Return the size of cpuset_t at the kernel level */
> +SYSCTL_INT(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
> + =A0 =A0 =A0 0, sizeof(cpuset_t), "sizeof(cpuset_t)");
> +
> =A0cpuset_t *cpuset_root;
>
> =A0/*
>
> Modified: head/sys/kern/sched_ule.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/kern/sched_ule.c =A0 Sun Oct 31 23:04:15 2010 =A0 =A0 =A0 =
=A0(r214610)
> +++ head/sys/kern/sched_ule.c =A0 Mon Nov =A01 00:42:25 2010 =A0 =A0 =A0 =
=A0(r214611)
> @@ -2713,7 +2713,6 @@ sysctl_kern_sched_topology_spec(SYSCTL_H
> =A0 =A0 =A0 =A0return (err);
> =A0}
>
> -static size_t _kern_cpuset_size =3D sizeof(cpuset_t);
> =A0#endif
>
> =A0SYSCTL_NODE(_kern, OID_AUTO, sched, CTLFLAG_RW, 0, "Scheduler");
> @@ -2751,14 +2750,6 @@ SYSCTL_PROC(_kern_sched, OID_AUTO, topol
> =A0 =A0 CTLFLAG_RD, NULL, 0, sysctl_kern_sched_topology_spec, "A",
> =A0 =A0 "XML dump of detected CPU topology");
>
> -/*
> - * Return the size of cpuset_t at the kernel level
> - *
> - * XXX (gcooper): replace ULONG with SIZE once CTLTYPE_SIZE is implement=
ed.
> - */
> -SYSCTL_ULONG(_kern_sched, OID_AUTO, cpusetsize, CTLFLAG_RD,
> - =A0 =A0&_kern_cpuset_size, 0, "Kernel-level cpuset_t struct size");
> -
> =A0#endif
>
> =A0/* ps compat. =A0All cpu percentages from ULE are weighted. */
>

--=20
wbr,
pluknet



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTi=TL4Yz5oR88qqorBvouWm0aYYmiQ>