tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Some changes to autoconfiguration APIs



> Date: Fri, 30 Apr 2021 18:14:36 -0700
> From: Jason Thorpe <thorpej%me.com@localhost>
> 
> > On Apr 25, 2021, at 4:26 AM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> > 
> > (It might be nice to see a worked example of how this improves the API
> > usage -- from a cursory skim of the branch's mechanical changes it's
> > not clear to me, but I expect you have some highlights or something
> > more in mind.)
> 
> If nothing else, you don't have to remember which order the
> arguments come in.  The new API also means you don't have to switch
> from e.g. config_found_ia() to config_found_sm_loc() just because
> you want to pass a new bit of info.

This seems like a trivial cosmetic advantage for a serious regression
in the type safety of a central kernel API?  I'm struggling to see the
value proposition for such a large rototill (other than the auditing
that it invited).

> > Here are two alternative approaches that also don't involve a
> > menagerie of config_{search,found}_xyzzy functions:
> > 
> > 1. C99 initializers:
> > 2. pthread_attr-style initialization functions:
> 
> I don't like these approaches as much for two reasons: aesthetic
> (they're more complicated in the simple cases), and that they
> increase the ABI surface.

If they're more complicated in simple cases, we could have
(type-safe!) shortcuts for the simple cases like config_found_ia...

Even if we had a stable kernel ABI (which we don't), we have ways to
make the ABI stable: we can use __RENAME, we can have struct cfargs be
a pointer to an opaque object allocated by cfargs_init and freed by
config_found* or a new cfargs_destroy, &c.  But we can also just do
what we've always done and bump the kernel version when the module ABI
changes.

I'm also not clear on why we need to make this extensible for every
bus that can't be done with bus-specific attach_args.  But my first
concern is that this is

(a) a regression from a type-safe API to a type-unsafe API for a
central part of the NetBSD driver interface, and

(b) a tree-wide change, applying to every bus in the tree, which, by
design, cannot be meaningfully compile-tested.

> It was REALLY obvious that there was a lot of blind copy-pasta
> lurking around as I audited everything.

I appreciate the work you did to audit this!  But we should make it
easier, not harder, for the compiler to audit mistakes if we're going
to make tree-wide changes; this appears to be strictly a regression on
technical grounds, for a change in cosmetics.  The same audit outcome
could have applied just as well to the type-safe(r) API we had.


Home | Main Index | Thread Index | Old Index