From owner-svn-src-all@FreeBSD.ORG Thu Jun 28 23:11:25 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C3CC11065693; Thu, 28 Jun 2012 23:11:25 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id A320B8FC08; Thu, 28 Jun 2012 23:11:25 +0000 (UTC) Received: from xp5k.my.domain (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q5SNBNOD022590; Thu, 28 Jun 2012 23:11:23 GMT (envelope-from listlog2011@gmail.com) Message-ID: <4FECE494.5020005@gmail.com> Date: Fri, 29 Jun 2012 07:11:16 +0800 From: David Xu User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:13.0) Gecko/20120628 Thunderbird/13.0.1 MIME-Version: 1.0 To: Kostik Belousov References: <201206272032.q5RKWjvt031174@svn.freebsd.org> <4FEBB8C9.8070006@gmail.com> <4FEBC0A2.3010708@gmail.com> <4FEBC70F.40408@gmail.com> <20120628075301.GS2337@deviant.kiev.zoral.com.ua> <4FEC1AAB.6070408@gmail.com> <4FEC557F.4080807@gmail.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Attilio Rao , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, davidxu@freebsd.org Subject: Re: svn commit: r237660 - head/lib/libc/gen X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@freebsd.org List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jun 2012 23:11:25 -0000 On 2012/06/28 21:52, Kostik Belousov wrote: > On Thu, Jun 28, 2012 at 4:00 PM, David Xu wrote: >> On 2012/6/28 16:49, David Xu wrote: >>> On 2012/6/28 15:53, Konstantin Belousov wrote: >>>> On Thu, Jun 28, 2012 at 10:53:03AM +0800, David Xu wrote: >>>>> On 2012/6/28 10:32, Attilio Rao wrote: >>>>>> 2012/6/28, David Xu: >>>>>>> On 2012/6/28 10:21, Attilio Rao wrote: >>>>>>>> 2012/6/28, David Xu: >>>>>>>>> On 2012/6/28 4:32, Konstantin Belousov wrote: >>>>>>>>>> Author: kib >>>>>>>>>> Date: Wed Jun 27 20:32:45 2012 >>>>>>>>>> New Revision: 237660 >>>>>>>>>> URL: http://svn.freebsd.org/changeset/base/237660 >>>>>>>>>> >>>>>>>>>> Log: >>>>>>>>>> Optimize the handling of SC_NPROCESSORS_CONF, by using auxv >>>>>>>>>> AT_NCPU >>>>>>>>>> value if present. >>>>>>>>>> >>>>>>>>>> MFC after: 1 week >>>>>>>>>> >>>>>>>>>> Modified: >>>>>>>>>> head/lib/libc/gen/sysconf.c >>>>>>>>>> >>>>>>>>>> Modified: head/lib/libc/gen/sysconf.c >>>>>>>>>> >>>>>>>>>> ============================================================================== >>>>>>>>>> --- head/lib/libc/gen/sysconf.c Wed Jun 27 20:24:25 2012 >>>>>>>>>> (r237659) >>>>>>>>>> +++ head/lib/libc/gen/sysconf.c Wed Jun 27 20:32:45 2012 >>>>>>>>>> (r237660) >>>>>>>>>> @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$"); >>>>>>>>>> #include >>>>>>>>>> #include >>>>>>>>>> >>>>>>>>>> +#include >>>>>>>>>> #include >>>>>>>>>> #include >>>>>>>>>> #include >>>>>>>>>> @@ -51,6 +52,7 @@ __FBSDID("$FreeBSD$"); >>>>>>>>>> >>>>>>>>>> #include "../stdlib/atexit.h" >>>>>>>>>> #include "tzfile.h" /* from >>>>>>>>>> ../../../contrib/tzcode/stdtime */ >>>>>>>>>> +#include "libc_private.h" >>>>>>>>>> >>>>>>>>>> #define _PATH_ZONEINFO TZDIR /* from tzfile.h */ >>>>>>>>>> >>>>>>>>>> @@ -585,6 +587,8 @@ yesno: >>>>>>>>>> >>>>>>>>>> case _SC_NPROCESSORS_CONF: >>>>>>>>>> case _SC_NPROCESSORS_ONLN: >>>>>>>>>> + if (_elf_aux_info(AT_NCPUS,&value, sizeof(value)) == 0) >>>>>>>>>> + return ((long)value); >>>>>>>>>> mib[0] = CTL_HW; >>>>>>>>>> mib[1] = HW_NCPU; >>>>>>>>>> break; >>>>>>>>>> >>>>>>>>> Will this make controlling the number of CPU online or CPU hotplug >>>>>>>>> be impossible on FreeBSD ? >>>>>>>> If I think about hotplug CPUs I can think of other 1000 >>>>>>>> problems/races/bad situations to be fixed before this one, really. >>>>>>> These are problems only in kernel, but kib's change is about ABI >>>>>>> between userland and kernel, I hope we don't introduce an ABI which >>>>>>> is not extendable road stone. >>>>>> I'm not entirely sure I see the ABI breakage here. >>>>> It is not breakage, it is the ABI thinks number of online cpu is fixed, >>>>> obviously, it is not the case in future unless FreeBSD won't support >>>>> dynamic number of online cpus. >>>>> >>>>> >>>>>> If the AT_NCPUS >>>>>> becames unconvenient and not correct at some point we can just fix >>>>>> sysconf() to not look into the aux vector anymoe. >>>>> If you already know this will be a problem, why do you introduce it >>>>> and later need to fix it ? >>>>> >>>>>> Please note that >>>>>> AT_NCPUS is already exported nowadays. I think this is instead a >>>>>> clever optimization to avoid the sysctl() (usual way to retrieve the >>>>>> number of CPUs). >>>>> But why don't you cache it in libc ? following code is enough: >>>>> >>>>> static int online_cpu; >>>>> if (online_cpu == 0) >>>>> online_cpu = sysctl >>>>> return online_cpu; >>>>> >>>> Thread did evolved somewhat while I was AFK. >>>> >>>> First, please note that the ABI which I designed there is fixable: >>>> if kernel does not export AT_NCPUS at all, then auxv correctly handles >>>> the situation returning an error, and libc falls back to sysctl(2). >>> >>> Do we really want to bypass sysctl and instead passing all info via auxv >>> vector ? >>> I found the sysconf() is a bunch of switch-case, which is already slow, >>> before >>> _SC_NPROCESSES_ONLN, it has already a quite number of case branches, >>> and in your code, it calls _elf_aux_info() which also has some >>> switch-cases branch, >>> if you cache smp_cpus in libc, the call for _elf_aux_info is not needed, >>> and you >>> don't need code in kernel to passing it either, in any case, the code to >>> call >>> sysctl is still needed, so why don't we just use sysctl instead and cache >>> the result in libc ? this at least can generate small code and a bit >>> faster after >>> first call to sysconf(_SC_NPROCESSES_ONLN). >> >> And as a side note, I think we should not put non-critical code into >> fork/exec >> path, these two functions are rather critical path for any UNIX like >> system, >> anything slowing down these two functions will affect overall performance, >> so we should not waste cpu cycle trying to push data to user mode via auxv >> or other ways and yet the data is not used by user code in most time, >> such as the number of online cpu. And in rtld-elf or libc, we should not >> waste >> too much time before executing main(). > My motivation for extending auxv vector and to develop auxv.c was > exactly to shave around dozen > of syscalls from the application startup sequence. If you look at the > ktrace output of the binary startup > on RELENG_8 libc, you should note exactly the sysctls to request > ncpus, pagesizes, canary and so on. > In RELENG_9 and HEAD, there are no sysctls in the trace, because the > data is already pre-filled by > kernel for libc consumption. > The sysconf(3) commit you are commenting on was caused by jemalloc(3) > initialization starting using > _sysconf(3) to query ncpu (for older version, which used sysctl > directly, I direct auxv call). So HEAD > has temporary +1 sysctl syscall done on app startup, now it should be > back to zero. This is a bug of jemalloc, in case __isthreaded is zero, it should not call sysctl to get number of cpu. I think it has already bypassed pthread_mutex_lock/unlock if __isthreaded is zero. > > In other words, 'we should not put non-critical code into fork/exec > path' exactly contradicts with > proposal to remove auxv, since exec would need to call sysctls to > fetch the same data. >