Date: Wed, 6 Mar 2013 07:11:19 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Marius Strobl <marius@alchemy.franken.de> Cc: arch@freebsd.org Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306061119.GA77841@onelab2.iet.unipi.it> In-Reply-To: <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> <20130306010720.GA68018@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130306061119.GA77841>