revising sys/conf/files* dependencies

[Luigi Rizzo]

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