Date: Wed, 15 Jun 2011 10:39:34 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Sergey Kandaurov <pluknet@freebsd.org> Cc: Attilio Rao <attilio@freebsd.org>, FreeBSD Current <freebsd-current@freebsd.org>, David Xu <davidxu@freebsd.org>, Bruce Evans <bde@freebsd.org> Subject: Re: svn commit: r214611 - head/sys/kern Message-ID: <20110615073934.GJ48734@deviant.kiev.zoral.com.ua> In-Reply-To: <BANLkTim_4HYekFgBWXp75qDoVOf7dnjEyw@mail.gmail.com> References: <201011010042.oA10gPeg016326@svn.freebsd.org> <BANLkTi=TL4Yz5oR88qqorBvouWm0aYYmiQ@mail.gmail.com> <4DF816E4.1080003@freebsd.org> <BANLkTim_4HYekFgBWXp75qDoVOf7dnjEyw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--3gYN3UO91xj0Or/Q Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 15, 2011 at 09:41:28AM +0400, Sergey Kandaurov wrote: > On 15 June 2011 06:20, David Xu <davidxu@freebsd.org> wrote: > > On 2011/06/14 20:02, Sergey Kandaurov wrote: > >> On 1 November 2010 03:42, David Xu <davidxu@freebsd.org> wrote: > >>> Author: davidxu > >>> Date: Mon Nov =9A1 00:42:25 2010 > >>> New Revision: 214611 > >>> URL: http://svn.freebsd.org/changeset/base/214611 > >>> > >>> Log: > >>> =9AUse integer for size of cpuset, as it won't be bigger than INT_MAX, > >>> =9AThis is requested by bge. > >>> =9AAlso move the sysctl into file kern_cpuset.c, because it should > >>> =9Aalways be there, it is independent of thread scheduler. > >> > >> Hi. > >> This breaks for me fetching a cpusetsize value with sysconf(3) interfa= ce, > >> as after this change sysconf(3) consumers expect a long return type, w= hile > >> sysctl kern.sched.cpusetsize has switched from long to int type in ker= nel. > >> 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: > >> =9A =9A =9A =9A long lvalue; > >> =9A =9A =9A =9A size_t len; > >> > >> =9A =9A =9A =9A len =3D sizeof(lvalue); > >> =9A =9A =9A =9A if (sysctlbyname("kern.sched.cpusetsize", &lvalue, &le= n, NULL, 0) < 0) > >> =9A =9A =9A =9A =9A =9A =9A =9A err(1, "sysctlbyname"); > >> =9A =9A =9A =9A printf("sysctl: %ld\n", lvalue); > >> =9A =9A =9A =9A printf("sysctl: %d -- explicitly casted to (int)\n", (= int)lvalue); > >> =9A =9A =9A =9A printf("sysconf: %ld\n", sysconf(_SC_CPUSET_SIZE)); > >> =9A =9A =9A =9A 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 cpuse= tsize > >> 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 =9A =9A =9A (revision 223073) > >> +++ lib/libkvm/kvm_pcpu.c =9A =9A =9A (working copy) > >> @@ -120,7 +120,7 @@ > >> =9Avoid * > >> =9Akvm_getpcpu(kvm_t *kd, int cpu) > >> =9A{ > >> - =9A =9A =9A long kcpusetsize; > >> + =9A =9A =9A int kcpusetsize; > >> =9A =9A =9A =9A ssize_t nbytes; > >> =9A =9A =9A =9A uintptr_t readptr; > >> =9A =9A =9A =9A 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 > >> > >> > > Try this patch, I think it should fix it. > > > > Index: lib/libc/gen/sysconf.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/libc/gen/sysconf.c =9A =9A =9A(revision 221356) > > +++ lib/libc/gen/sysconf.c =9A =9A =9A(working copy) > > @@ -599,11 +599,11 @@ > > > > =9A#ifdef _SC_CPUSET_SIZE > > =9A =9A =9A =9Acase _SC_CPUSET_SIZE: > > - =9A =9A =9A =9A =9A =9A =9A len =3D sizeof(lvalue); > > - =9A =9A =9A =9A =9A =9A =9A if (sysctlbyname("kern.sched.cpusetsize",= &lvalue, &len, NULL, > > + =9A =9A =9A =9A =9A =9A =9A len =3D sizeof(value); > > + =9A =9A =9A =9A =9A =9A =9A if (sysctlbyname("kern.sched.cpusetsize",= &value, &len, NULL, > > =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A0) =3D=3D -1) > > =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9Areturn (-1); > > - =9A =9A =9A =9A =9A =9A =9A return (lvalue); > > + =9A =9A =9A =9A =9A =9A =9A return ((long)(value)); > > =9A#endif > > > > =9A =9A =9A =9Adefault: > > >=20 > Great, thanks! Look good for me. > Nitpicking: > return ((long)value); should be enough (extra parenthesis). This patch accomodates the userland to the changed ABI. Why it was changed at all ? I would argue that keeping the stable ABI there is more important then using a 'clean' type. At least, the stable branches usermode is broken on the current kernel. --3gYN3UO91xj0Or/Q Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAk34YbYACgkQC3+MBN1Mb4gUCACcDZfpHQsPnocuWspzIwu/940l 5HsAoKlnlXmYScXlA73zTwW22w6AAALu =geLd -----END PGP SIGNATURE----- --3gYN3UO91xj0Or/Q--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110615073934.GJ48734>