Re: CVS commit: src/sys/opencrypto

[Taylor R Campbell <campbell+netbsd-source-changes-d%mumble.net@localhost>]

   Date: Mon, 27 Jan 2014 05:45:32 -0800 (PST)
   From: Paul Goyette <paul%whooppee.com@localhost>

   Taylor R Campbell wrote:

   > Would it suffice to add an open count to struct bdevsw and struct
   > cdevsw, to be maintained by {bdev,cdev}_{open,close}?

   I think this would be sufficient.

Minor snag: all the bdevsw/cdevsw structs are const, so it would have
to be maintained in a separate table.

   > A lesser problem is that the steps to finalize a device driver module
   > are complicated (attach/detach the devsw, the cfdriver, the cfattach,
   > the cfdata) and different drivers do them in a different order and may
   > or may not back out the same way (e.g., dtv(4) ignores devsw_detach
   > failure), so we ought to formalize what the right way to do this is
   > once we have a way that can work.

   That would definitely be appreciated.

   It would seem that config_{init,fini}_component() are intended to handle 
   much/most of this, but these routines are not documented and not used 
   universally. (see sys/kern/subr_autoconf.c)

It looks like MODULE_CMD_INIT should do (other initialization and
then) config_init_component and then devsw_attach last, and
MODULE_CMD_FINI should do devsw_detach first and then
config_fini_component (and then other finalization now that none of
the resources can be in use by the userland or by the kernel).

That way, on attach, the device won't be visible to userland until it
is hooked up to autoconf, and on detach, if the device is open in
userland, devsw_detach will fail and prevent the other destruction.

I believe devsw_detach and config_fini_component are idempotent, so if
for some reason config_fini_component fails the first time around, it
need not back out by doing devsw_attach again.  On the other hand,
that might make diagnosis of the unload failure difficult because
userland can no longer talk to the device.

Perhaps we ought to combine config_init_component and devsw_attach to
avoid having to write this pile of code like I wrote for drm2:

case MODULE_CMD_INIT:
        error = config_init_component(cfdriver_ioconf_drm, cfattach_ioconf_drm,
            cfdata_ioconf_drm); 
        if (error) {
                aprint_error("drm: unable to init config component: %d\n",
                    error);
                goto init_fail0;
        }
        error = devsw_attach("drm", NULL, &bmajor, &drm_cdevsw, &cmajor);
        if (error) {
                aprint_error("drm: unable to attach devsw: %d\n", error);
                goto init_fail1;
        }
        return 0;

        /* These had better not fail...  */
init_fail2: __unused
        (void)devsw_detach(NULL, &drm_cdevsw);
init_fail1:
        (void)config_fini_component(cfdriver_ioconf_drm, cfattach_ioconf_drm,
            cfdata_ioconf_drm);
init_fail0:
        return error;

case MODULE_CMD_FINI:
        error = devsw_detach(NULL, &drm_cdevsw);
        if (error)
                return error;
        error = config_fini_component(cfdriver_ioconf_drm, cfattach_ioconf_drm,
            cfdata_ioconf_drm);
        if (error)
                return error;
        return 0;