From owner-cvs-src@FreeBSD.ORG Wed Oct 17 13:42:50 2007 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3945C16A469; Wed, 17 Oct 2007 13:42:50 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 0D35A13C494; Wed, 17 Oct 2007 13:42:49 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by elvis.mu.org (Postfix) with ESMTP id 0AB9C1A4D80; Wed, 17 Oct 2007 06:16:45 -0700 (PDT) From: John Baldwin To: "Constantine A. Murenin" Date: Wed, 17 Oct 2007 09:03:30 -0400 User-Agent: KMail/1.9.7 References: <200710161809.10755.jhb@freebsd.org> <47153F62.1080709@FreeBSD.org> In-Reply-To: <47153F62.1080709@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200710170903.31724.jhb@freebsd.org> Cc: src-committers@freebsd.org, Alexander Leidinger , cvs-src@freebsd.org, Poul-Henning Kamp , cvs-all@freebsd.org, "Constantine A. Murenin" , Wilko Bulte Subject: Re: cvs commit: src/etc Makefile sensorsd.conf src/etc/defaults rc.conf src/etc/rc.d Makefile sensorsd src/lib/libc/gen sysctl.3 src/sbin/sysctl sysctl.8 sysctl.c src/share/man/man5 rc.conf.5 src/share/man/man9 Makefile sensor_attach.9 src/sys/conf f X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Oct 2007 13:42:50 -0000 On Tuesday 16 October 2007 06:46:58 pm Constantine A. Murenin wrote: > On 16/10/2007 18:09, John Baldwin wrote: > > > On Tuesday 16 October 2007 05:46:18 pm Constantine A. Murenin wrote: > > > >>On 16/10/2007, John Baldwin wrote: > >> > >>>On Tuesday 16 October 2007 12:33:11 pm Alexander Leidinger wrote: > >>> > >>>>Constantine asked for review several times on -current. He got some > >>>>reviews several times for commits to perforce. He incorporated > >>>>suggestions from those reviews, or explained why it is like it is and > >>>>why he can not switch (with no replies with suggestions how to solve > >>>>the problems he sees with the suggestions). Now you come and ask why > >>>>nobody pointed out some flaws before (without telling us which > >>>>technical flaws you talk about). > >>> > >>>At least from my point of view this is not quite accurate as pretty much > > > > all > > > >>>my feedback to the p4 commits was ignored with basically "Well, I don't > > > > like > > > >>>doing it that way". Specifically, with regards to creating dynamic sysctl > >>>trees, Constantine feels that sysctl_add_oid(9) is a hack rather than > >>>recognizing that this is a feature of FreeBSD's sysctl system despite > >>>repeated e-mails on the subject. > >> > >>Dear John, > >> > >>I have specifically addressed this concern of yours just several weeks ago. > >> > >>Please see the following message, which you've never replied to: > >> > >>http://lists.freebsd.org/pipermail/p4-projects/2007-September/021121.html > >> > >>I've used the documented parts of the FreeBSD's sysctl interface to > >>preserve 100% userland compatibility with OpenBSD. > > > > > > FreeBSD already provides an interface for creating dynamic sysctl trees that > > is simpler than having to simulate a pseudo-tree via the .oid_handler > > routine. In some cases (such as kern.proc) FreeBSD still uses a function > > handler rather than giving each process its own sysctl node. However, in > > other cases (generally more recent ones, and ones not as large/performance > > impacting) such as dev.* or the recent proposal to give ifnet's their own > > nodes under 'net.if' or the like, sysctl_add_oid(9) is used. > > Which one is simpler is questionable. Take one more look at the number > of different macros that are available in the sysctl(9) and friends, and > then compare with four functions of the sensors framework: > > * sensor_attach(9) to attach a sensor to a sensordev tree > * sensordev_install(9) to register the sensordev tree with sysctl(9) > and the same with the "de" prefix > > sensordev_install(9) acts similarly to the sysctl(9) ctx concept, but is > geared towards the sensors approach > > If you want to make sure you don't attract any new contributors, then > certainly the bunch of the sysctl(9) macros are simpler. ;) I never said to not have sensor_*() routines. I just think that the sensor implementation should make use of dynamic sysctls to build the sysctl interface rather than treating dynamic sysctls as a temporary hack and adding a duplicate interface for walking the hw.sensors tree. > > As to the process of walking sysctl trees being undocumented, it is simply > > missing a wrapper routine ala sysctlbyname(3) and a manpage. You could > > easily provide one and thus provide a facility for enumerating many different > > things than having several one-off oid_handler routines to enumerate > > subtrees. However, it is not some "backdoor" hack interface anymore than > > sysctlbyname(3) is. They are both equally hackish or non-hackish (depending > > on your point of view). > > This interface is about having a special tree for the hardware sensors, > with special rules regarding the namespace. What you suggest is that I > try to abandon this, but then why do you need this framework to start with? I suggest you think in layered abstractions. sysctl is a tool for creating a namespace that the kernel exports to userland. Let sysctl manage the namespace and the sensor code just populate nodes in the namespace and manage sensors. I did some thinking last night about what I would really want out of a sensors framework btw, and will send a mail to arch@ about that. The short version though is that I do think the existing sensors code has a lot of things right (The sensor structure seems very reasonable for example. I might have done 4 states rather than 3, but 3 is quite workable as is.) The major hangups I think are requiring sensors to be a kernel user interface rather than a userland interface and the idea that that is somehow more secure, but more on that in my e-mail to arch@. -- John Baldwin