From owner-freebsd-arch@FreeBSD.ORG Wed Mar 6 01:07:23 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 80378677 for ; Wed, 6 Mar 2013 01:07:23 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 12E38871 for ; Wed, 6 Mar 2013 01:07:22 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.6/8.14.6/ALCHEMY.FRANKEN.DE) with ESMTP id r2617K8L069532; Wed, 6 Mar 2013 02:07:20 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.6/8.14.6/Submit) id r2617KrT069531; Wed, 6 Mar 2013 02:07:20 +0100 (CET) (envelope-from marius) Date: Wed, 6 Mar 2013 02:07:20 +0100 From: Marius Strobl To: Warner Losh Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306010720.GA68018@alchemy.franken.de> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 01:07:23 -0000 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". > >> I'd say those are correct: don't compile if_rl unless you have both rl and pci in the kernel.... There's no misunderstanding here: people know what it means. > > > >>> So, as said in the summary, I'd like to modify these and similar > >>> lines so that the error notification comes early; normally > >>> this is achieved by removing the bus name. > >> > >> That might be OK. The original intent for this form was to allow one to remove all pci drivers from the kernel by just removing the device pci line from the config file. This is at best a dubious feature, experience has shown. I'm worried that "fixing" it will produce a worse problem than the one you are fixing. > > > > if that is the intent, at the very least we should explain it > > in the file so it is clear why "pci" and "scbus" are handled > > differently. But apart form the convenience for developers > > (and given we have "include", this can be easily overcome), > > the "fix" i propose should only point out broken config files. > > They aren't handled differently. You've always had to have all the dependencies to make things work. > > The fix you propose moves the problem around, and won't solve the problem for those devices that have multiple attachments. Since that has gone rare, that might be an OK problem exchange. > > However, it would be better to have required dependencies be codified somewhere in the files.*. Not having that, your solution sucks the least to accomplish the goals that conflict with the original design that happened to be a bad design point in the first place.... > > So yea, it's fine, but the real solution isn't to solve these problems by hacking the current system, but rather by finishing the module system that has always been totally half-assed. To finish the system, you likely have to ditch config(8) as we know it. > 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. Marius