Date: Wed, 6 Mar 2013 09:34:21 +0100 From: Marius Strobl <marius@alchemy.franken.de> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: arch@freebsd.org Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306083420.GJ955@alchemy.franken.de> In-Reply-To: <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> <20130306061119.GA77841@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Mar 06, 2013 at 07:11:19AM +0100, Luigi Rizzo wrote:
> 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.
Correct, as Warner already said, but what you propose isn't an
acceptable workaround.
>
> 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.
Like Warner I believe that most developers are very well aware how
#1 actually behaves.
>
> 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...
>
Wrong, it's not of your category #2 as building sym_hipd.o statically
into the kernel doesn't cause a link error when "device pci" is
omitted - please give it a try. As such it is a very relevant example
of PCI drivers that don't use any PCI specialties like things generated
from pci_if.m or functions in sys/dev/pci/*.c. For hardware busses
besides PCI it's even more common that general interfaces like that
of newbus are sufficient and omitting the respective "device foobus"
doesn't cause a link error when building a device driver attaching
to that foobus into the kernel.
What's actually not relevant in your examples is scbus. This is due
to the fact that "device scbus" actually means CAM, which is neither
a hardware bus (well, we also use scbus to express the physical link
between HBA and target, but the main thing is that it drags in all
of CAM) nor newbus'ified and as such provides it's own API and ABI.
Thus any HBA driver is almost guaranteed to need to call a function
of CAM only present when "device scbus" is also present.
Marius
Marius
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130306083420.GJ955>