From owner-freebsd-current@FreeBSD.ORG  Wed Jun 15 05:41:30 2011
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 1769F106564A;
	Wed, 15 Jun 2011 05:41:30 +0000 (UTC)
	(envelope-from pluknet@gmail.com)
Received: from mail-qy0-f182.google.com (mail-qy0-f182.google.com
	[209.85.216.182])
	by mx1.freebsd.org (Postfix) with ESMTP id 7D5ED8FC08;
	Wed, 15 Jun 2011 05:41:29 +0000 (UTC)
Received: by qyk27 with SMTP id 27so39943qyk.13
	for <multiple recipients>; Tue, 14 Jun 2011 22:41:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;
	h=domainkey-signature:mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type
	:content-transfer-encoding;
	bh=iaGbDQBzdbowpnO0iQr/uN10nK1VY6oLYvLtC1rCMGs=;
	b=iGJYPhVaI8ZPd6+TGfLZD3xVS6ZsxL3XiI++JCw+fP5QtlLQmM7cuxHBzXVjXhC9cF
	DIgSdCKUX/jqoSS+TDI55DkEBldLNrPRuYsTyxAzggQha5wY+WJ5Uzn943sKw+tj0Kda
	32kB3jTn1IfeB3z6sUkk7qPvq/31bhl+eJJ7w=
DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;
	h=mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type
	:content-transfer-encoding;
	b=EDKLIGzgG62mnfmV3spw4SozxMVjr8CgBKaTx4Q8SmJyAF/FFpb+nTvVZQt9J2CglY
	9B+PF4ImnbEOcYmk5LvYHa1LhsfDvynhkOKaunQhHmNHBVoHAPx8RNRxWddHOuP2iKKk
	rXT38aldHDKhQEvlQKivNVeuoGU6CQM/oo6Do=
MIME-Version: 1.0
Received: by 10.229.35.1 with SMTP id n1mr60809qcd.84.1308116488707; Tue, 14
	Jun 2011 22:41:28 -0700 (PDT)
Sender: pluknet@gmail.com
Received: by 10.229.99.197 with HTTP; Tue, 14 Jun 2011 22:41:28 -0700 (PDT)
In-Reply-To: <4DF816E4.1080003@freebsd.org>
References: <201011010042.oA10gPeg016326@svn.freebsd.org>
	<BANLkTi=TL4Yz5oR88qqorBvouWm0aYYmiQ@mail.gmail.com>
	<4DF816E4.1080003@freebsd.org>
Date: Wed, 15 Jun 2011 09:41:28 +0400
X-Google-Sender-Auth: T0ZGkUrDzLhtEYt2EpQQXGQQllQ
Message-ID: <BANLkTim_4HYekFgBWXp75qDoVOf7dnjEyw@mail.gmail.com>
From: Sergey Kandaurov <pluknet@freebsd.org>
To: David Xu <davidxu@freebsd.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: Attilio Rao <attilio@freebsd.org>,
	FreeBSD Current <freebsd-current@freebsd.org>,
	Bruce Evans <bde@freebsd.org>
Subject: Re: svn commit: r214611 - head/sys/kern
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
	<freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>, 
	<mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
	<mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 15 Jun 2011 05:41:30 -0000

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 =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, whi=
le
>> sysctl kern.sched.cpusetsize has switched from long to int type in kerne=
l.
>> 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:
>> =A0 =A0 =A0 =A0 long lvalue;
>> =A0 =A0 =A0 =A0 size_t len;
>>
>> =A0 =A0 =A0 =A0 len =3D sizeof(lvalue);
>> =A0 =A0 =A0 =A0 if (sysctlbyname("kern.sched.cpusetsize", &lvalue, &len,=
 NULL, 0) < 0)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "sysctlbyname");
>> =A0 =A0 =A0 =A0 printf("sysctl: %ld\n", lvalue);
>> =A0 =A0 =A0 =A0 printf("sysctl: %d -- explicitly casted to (int)\n", (in=
t)lvalue);
>> =A0 =A0 =A0 =A0 printf("sysconf: %ld\n", sysconf(_SC_CPUSET_SIZE));
>> =A0 =A0 =A0 =A0 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 cpusets=
ize
>> 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 =A0 =A0 =A0 (revision 223073)
>> +++ lib/libkvm/kvm_pcpu.c =A0 =A0 =A0 (working copy)
>> @@ -120,7 +120,7 @@
>> =A0void *
>> =A0kvm_getpcpu(kvm_t *kd, int cpu)
>> =A0{
>> - =A0 =A0 =A0 long kcpusetsize;
>> + =A0 =A0 =A0 int kcpusetsize;
>> =A0 =A0 =A0 =A0 ssize_t nbytes;
>> =A0 =A0 =A0 =A0 uintptr_t readptr;
>> =A0 =A0 =A0 =A0 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 =A0 =A0 =A0(revision 221356)
> +++ lib/libc/gen/sysconf.c =A0 =A0 =A0(working copy)
> @@ -599,11 +599,11 @@
>
> =A0#ifdef _SC_CPUSET_SIZE
> =A0 =A0 =A0 =A0case _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 len =3D sizeof(value);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (sysctlbyname("kern.sched.cpusetsize", &=
value, &len, NULL,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00) =3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (-1);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (lvalue);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return ((long)(value));
> =A0#endif
>
> =A0 =A0 =A0 =A0default:
>

Great, thanks! Look good for me.
Nitpicking:
 return ((long)value); should be enough (extra parenthesis).

--=20
wbr,
pluknet