Date: Thu, 28 Jun 2012 16:52:50 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: davidxu@freebsd.org Cc: Attilio Rao <attilio@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r237660 - head/lib/libc/gen Message-ID: <CAFKVNxBM-Gqj7xwPsjz6LbN0j9MuB6fFPjSwimGipd61WKYDcA@mail.gmail.com> In-Reply-To: <4FEC557F.4080807@gmail.com> References: <201206272032.q5RKWjvt031174@svn.freebsd.org> <4FEBB8C9.8070006@gmail.com> <CAJ-FndBdpP4hFPyqHaU6GucXeGi3WT4d%2Bpu-9hCNEpAPZAZQgg@mail.gmail.com> <4FEBC0A2.3010708@gmail.com> <CAJ-FndD9Hc_KvHe8vC3usVDZh7MYb51gtXsHgmins8dBC3mu_w@mail.gmail.com> <4FEBC70F.40408@gmail.com> <20120628075301.GS2337@deviant.kiev.zoral.com.ua> <4FEC1AAB.6070408@gmail.com> <4FEC557F.4080807@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Jun 28, 2012 at 4:00 PM, David Xu <listlog2011@gmail.com> 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<listlog2011@gmail.com>: >>>>>> >>>>>> On 2012/6/28 10:21, Attilio Rao wrote: >>>>>>> >>>>>>> 2012/6/28, David Xu<listlog2011@gmail.com>: >>>>>>>> >>>>>>>> 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: >>>>>>>>> =A0 =A0 Optimize the handling of SC_NPROCESSORS_CONF, by using au= xv >>>>>>>>> =A0 =A0 AT_NCPU >>>>>>>>> =A0 =A0 value if present. >>>>>>>>> >>>>>>>>> =A0 =A0 MFC after: =A0 =A01 week >>>>>>>>> >>>>>>>>> Modified: >>>>>>>>> =A0 =A0 head/lib/libc/gen/sysconf.c >>>>>>>>> >>>>>>>>> Modified: head/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=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D >>>>>>>>> --- head/lib/libc/gen/sysconf.c =A0 =A0Wed Jun 27 20:24:25 2012 >>>>>>>>> (r237659) >>>>>>>>> +++ head/lib/libc/gen/sysconf.c =A0 =A0Wed Jun 27 20:32:45 2012 >>>>>>>>> (r237660) >>>>>>>>> @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$"); >>>>>>>>> =A0 =A0#include<sys/resource.h> >>>>>>>>> =A0 =A0#include<sys/socket.h> >>>>>>>>> >>>>>>>>> +#include<elf.h> >>>>>>>>> =A0 =A0#include<errno.h> >>>>>>>>> =A0 =A0#include<limits.h> >>>>>>>>> =A0 =A0#include<paths.h> >>>>>>>>> @@ -51,6 +52,7 @@ __FBSDID("$FreeBSD$"); >>>>>>>>> >>>>>>>>> =A0 =A0#include "../stdlib/atexit.h" >>>>>>>>> =A0 =A0#include "tzfile.h" =A0 =A0 =A0 =A0/* from >>>>>>>>> =A0 =A0../../../contrib/tzcode/stdtime */ >>>>>>>>> +#include "libc_private.h" >>>>>>>>> >>>>>>>>> =A0 =A0#define =A0 =A0_PATH_ZONEINFO =A0 =A0TZDIR =A0 =A0/* from = tzfile.h */ >>>>>>>>> >>>>>>>>> @@ -585,6 +587,8 @@ yesno: >>>>>>>>> >>>>>>>>> =A0 =A0 =A0 =A0case _SC_NPROCESSORS_CONF: >>>>>>>>> =A0 =A0 =A0 =A0case _SC_NPROCESSORS_ONLN: >>>>>>>>> + =A0 =A0 =A0 =A0if (_elf_aux_info(AT_NCPUS,&value, sizeof(value)= ) =3D=3D 0) >>>>>>>>> + =A0 =A0 =A0 =A0 =A0 =A0return ((long)value); >>>>>>>>> =A0 =A0 =A0 =A0 =A0 =A0mib[0] =3D CTL_HW; >>>>>>>>> =A0 =A0 =A0 =A0 =A0 =A0mib[1] =3D HW_NCPU; >>>>>>>>> =A0 =A0 =A0 =A0 =A0 =A0break; >>>>>>>>> >>>>>>>> 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 ? >>>> >>>>> =A0Please 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 =3D=3D 0) >>>> =A0 =A0 online_cpu =3D 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, =A0it 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 cach= e >> 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, =A0these 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 aux= v > 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. 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFKVNxBM-Gqj7xwPsjz6LbN0j9MuB6fFPjSwimGipd61WKYDcA>