From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 06:10:43 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 3329F302 for ; Wed, 6 Mar 2013 06:10:43 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 836852E5 for ; Wed, 6 Mar 2013 06:10:42 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id F3A257300A; Wed, 6 Mar 2013 07:11:19 +0100 (CET) Date: Wed, 6 Mar 2013 07:11:19 +0100 From: Luigi Rizzo To: Marius Strobl Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306061119.GA77841@onelab2.iet.unipi.it> References: <20130305083817.GD13187@onelab2.iet.unipi.it> <112844CF-69C3-49A3-8581-8EF2A7DA8E8A@bsdimp.com> <20130305211953.GA51357@onelab2.iet.unipi.it> <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> <20130306010720.GA68018@alchemy.franken.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130306010720.GA68018@alchemy.franken.de> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Mar 2013 06:10:43 -0000 On Wed, Mar 06, 2013 at 02:07:20AM +0100, Marius Strobl wrote: > On Tue, Mar 05, 2013 at 02:33:41PM -0700, Warner Losh wrote: > > > > On Mar 5, 2013, at 2:19 PM, Luigi Rizzo wrote: > > > > > On Tue, Mar 05, 2013 at 08:15:30AM -0700, Warner Losh wrote: > > >> > > >> On Mar 5, 2013, at 1:38 AM, Luigi Rizzo wrote: > > >> > > >>> Short Summary: > > >>> > > >>> I would like to revise sys/conf/files* and fix many erroneous usages of > > >>> > > >>> some/file.c optional foo_dev bar_bus > > >>> > > >>> changing them into one of the following > > >>> > > >>> some/file.c optional foo_dev # link error if bar_bus is missing > > >>> some/file.cxi optional foo_dev | bar_bus # logical OR > > >>> > > >>> ---------- > > >>> Full description: > > >>> > > >>> I always thought (wrongly) that a line of the form > > >>> > > >>> some/file.c optional foo bar baz # 1 > > >>> > > >>> in sys/conf/files* meant that file.c is compiled in if _any_ of the > > >>> options is specified in the kernel config. But i was wrong, the > > >>> above means that _all_ options are require, and the correct syntax > > >>> for alternative options is > > >>> > > >>> some/file.c optional foo | bar | baz # 2 > > >>> > > >>> I believe that i am not alone in this misunderstanding, and that > > >>> most entries in sys/conf/files* use form #1 in a wrong way, e.g.: > > >>> > > >>> dev/hptiop/hptiop.c optional hptiop scbus > > >>> dev/iscsi/initiator/iscsi.c optional iscsi_initiator scbus > > >>> dev/mfi/mfi_cam.c optional mfip scbus > > >>> pci/viapm.c optional viapm pci > > >>> pci/intpm.c optional intpm pci > > >>> pci/if_rl.c optional rl pci > > >>> (there are many many more) > > >>> > > >>> In all these cases, if you forget the scbus or pci in the kernel > > >>> config, the driver is not compiled in but you only detect it at > > >>> compile time. I'd rather be notified of the error at kernel link time. > > > ^^^^^^^^^^^^ i meant "run time", and this is the main problem: > > > if you forget the bus in the kernel config, the build will silently > > > discard the entry, but you will only realize it when you actually > > > run the kernel. Yet, having "rl" alone is surely an error, and it > > > should be flagged as such at build time. > > > > Yea... You are wanting dependency checking that does not exist today, and for which the meta-data does not exist today. > > > > Like I said, the intent of the original feature was to disable large classes of things quickly. Nobody really does that, so that feature can go... It is likely half broken these days. > > > > Uhm, according to a quick test I'd say there definitely are drivers > missing dependencies on pci in sys/conf/files, but it certainly seems > way better than "half broken". Marius, just to clarify: sys/conf/files has no reasonable way to express dependencies. Take as an example the common case (more later) of a bus b with sources in sys/dev/b/... and devices d1, d2, ... dn with sources in sys/dev/d*/... All d* require b to be present in order to load. In the modules, you express this as MODULE_DEPEND(). Say you want to build a kernel with "device d1" but forget to include "device b". For sys/conf/files you have the following options: (1) silently ignore devices with missing dependencies. You will discover the misconfiguration at run time (or, you can consider this a feature to quickly remove all drivers that depend on a given bus) sys/dev/b/b_foo.c optional b sys/dev/b/b_bar.c optional b ... sys/dev/d1/d1_foo.c optional d1 b sys/dev/d1/d1_bar.c optional d1 b ... sys/dev/dn/dn_foo.c optional dn b sys/dev/dn/dn_bar_b.c optional dn b ... (2) omit the dependency, causing an error at link time sys/dev/b/b_foo.c optional b sys/dev/b/b_bar.c optional b ... sys/dev/d1/d1_foo.c optional d1 sys/dev/d1/d1_bar.c optional d1 ... sys/dev/dn/dn_foo.c optional dn sys/dev/dn/dn_bar_b.c optional dn ... (3) enforce the dependency sys/dev/b/b_foo.c optional b | d1 | d2 | ... | dn sys/dev/b/b_bar.c optional b | d1 | d2 | ... | dn ... sys/dev/d1/d1_foo.c optional d1 sys/dev/d1/d1_bar.c optional d1 ... sys/dev/dn/dn_foo.c optional dn sys/dev/dn/dn_bar_b.c optional dn ... Right now, some buses (e.g. pci) use #1; others (e.g. scbus) use #2; very few use #3 (which as you can see becomes unmanageable when the number of customers grows, or we have a chain of dependencies). Warner gave an explanation why #1 can be useful. Personally i prefer #2 so i can be notified sooner if my kernel config file has a missing dependency. My feeling is that many people (including myself up to a few weeks ago) believe #1 behaves as #3. As Warren mentioned, a proper solution requires replacing config (8) with something that is better suited to the task; but i am not suggesting to do this now, I just wanted to get opinions on whether #2 is considered significantly more useful than #1 to deserve a change. > I'd say the proposed "fix" is only a bad kludge as it doesn't even > solve the problem for drivers only having a PCI front-end. This is > due to the fact that removing pci for these will only cause a link > error iff these drivers use methods only compiled in when device pci > is present, f.e. pci_enable_busmaster(9). However, one of the nice > things about newbus is that the bus front-ends actually are rather > bus-agnostic (and thus a single front-end may attach to different > busses) and even using pci_get_{device,vendor}(9) doesn't cause a > link error in case device pci isn't present. One example of such a > PCI device driver compiling without device pci is sym(4). Yep, > that example is a bit bad as sym_hipd.c actually misses the pci > dependency in sys/conf/files. It's just the first driver I found > and all "simple enough" drivers for PCI devices should fall in the > same category. as you say this does not seem to be a relevant example :) And it uses #2 so it already in the form that i prefer... cheers luigi