Re: Module autounload proposal: opt-in, not opt-out

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

> Date: Mon, 8 Aug 2022 07:18:05 -0700 (PDT)
> From: Paul Goyette <paul%whooppee.com@localhost>
> 
> I suspect that modules are not being used very much.  After all, we
> include nearly everything in GENERIC kernels, so there's rarely any
> need to load modules.  (The major exception being dtrace, IIUC, and
> that's almost exclusively for developers.)  I'm probably one of the
> most ardent proponents of modules, regularly running a completely
> stripped-out kernel (minimal built-in module code).

It's true modules don't get a lot of use because most things outside
zfs and dtrace (and maybe soon amdgpu) don't really need them, but I
appreciate the work you've been doing to keep them exercised and
improve modularity.  While I don't a MODULAR kernel myself, I do try
to incrementally make the kernel API and ABI more modular -- e.g.,
moving struct device internals to a private header, fixing the
devsw_detach contract, and generally improving abstractions for safe
teardown.

> I'd like to suggest an alternative proposal: have a new sysctl(8)
> variable that controls whether the default is opt-in vs opt-out.
> Setting opt-in would require a module to explicitly return 0 from
> its modcmd(AUTOUNLOAD) routine, while setting opt-out would accept
> either 0 or ENOTTY (the current behavior).  I'm not sure which
> would be the best default (and won't argue, regardlesss of which
> default is selected), but the ability to retain the current opt-
> out behavior is important to me, and might lead to more/better
> testing (albeit mostly ad-hoc vs formal testing).

We can have a sysctl knob, but just keeping the unload path casually
exercised like this has limited utility, because unload is
qualitatively different from load:  Like detach, unload needs to be
able to gracefully handle with every possible state that all the
resources published or used by the module might be in.

Earlier this year I put a lot of work into making detach more
reliable.  Some of that work was hours of triggering heavy activity
and then physically yanking devices repeatedly, and then diagnosing
the crashes from time to time.  But it also required a thoughtful
audit of the states the specdev/specnode/autoconf instances can be in,
as well as driver-specific state.

Both types of work were necessary for making detach reliable -- and,
despite my attempts to make the existing APIs involved in detach work
reliably by default, there's still a lot of drivers out there with
buggy detach, but at least yanking a USB device is an explicit action
taken by an operator so it's easy to connect to the problems that tend
to arise.

Same deal with unload, except with autounload, the autoload might
happen behind the scenes and set a ten-second time-bomb you don't even
see connected to the original cause.

So we can add the sysctl knob, but we have to keep in mind that just
keeping the autounload path of any particular module exercised a
little is nowhere near enough to make it safe.