Date: Wed, 6 Mar 2013 02:07:20 +0100 From: Marius Strobl <marius@alchemy.franken.de> To: Warner Losh <imp@bsdimp.com> Cc: arch@freebsd.org Subject: Re: revising sys/conf/files* dependencies Message-ID: <20130306010720.GA68018@alchemy.franken.de> In-Reply-To: <6A93AA58-713D-4C25-A512-6927E27C5DE1@bsdimp.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130306010720.GA68018>