From 2d9d0a686485cd383daec7333aafd68f67bb02f4 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 12 Jan 2022 14:18:57 +0000 Subject: [PATCH 04/39] driver(9): New devsw members d_cfdriver, d_devtounit. If set, then bdev_open/cdev_open will use d_devtounit to map the dev_t to an autoconf instance (e.g., /dev/wd0a -> wd0) and hold a reference with device_lookup_acquire across the call to d_open. This guarantees that the autoconf instance cannot be detached while the devsw's d_open function is trying to open it (and also that the autoconf instance has finished *_attach before anyone can open it). Of course, if the underlying hardware has gone away, there will be I/O errors, but this avoids software synchronization bugs between open and detach for drivers that opt into it. It's up to the driver and bus to figure out how to deal with I/O errors from operations on hardware that has gone away while the software hasn't finished notifying everything that it's gone yet. XXX kernel ABI change to struct bdevsw/cdevsw requires bump --- sys/kern/subr_devsw.c | 49 +++++++++++++++++++++++++++++++++++++++++-- sys/sys/conf.h | 4 ++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/sys/kern/subr_devsw.c b/sys/kern/subr_devsw.c index 334328484a1a..d0451363a041 100644 --- a/sys/kern/subr_devsw.c +++ b/sys/kern/subr_devsw.c @@ -89,6 +89,7 @@ __KERNEL_RCSID(0, "$NetBSD: subr_devsw.c,v 1.38 2017/11/07 18:35:57 christos Exp #include #include #include +#include #ifdef DEVSW_DEBUG #define DPRINTF(x) printf x @@ -895,16 +896,38 @@ bdev_open(dev_t dev, int flag, int devtype, lwp_t *l) { const struct bdevsw *d; struct localcount *lc; - int rv, mpflag; + device_t dv = NULL/*XXXGCC*/; + int unit, rv, mpflag; d = bdevsw_lookup_acquire(dev, &lc); if (d == NULL) return ENXIO; + if (d->d_devtounit) { + /* + * If the device node corresponds to an autoconf device + * instance, acquire a reference to it so that during + * d_open, device_lookup is stable. + * + * XXX This should also arrange to instantiate cloning + * pseudo-devices if appropriate, but that requires + * reviewing them all to find and verify a common + * pattern. + */ + if ((unit = (*d->d_devtounit)(dev)) == -1) + return ENXIO; + if ((dv = device_lookup_acquire(d->d_cfdriver, unit)) == NULL) + return ENXIO; + } + DEV_LOCK(d); rv = (*d->d_open)(dev, flag, devtype, l); DEV_UNLOCK(d); + if (d->d_devtounit) { + device_release(dv); + } + bdevsw_release(d, lc); return rv; @@ -1050,16 +1073,38 @@ cdev_open(dev_t dev, int flag, int devtype, lwp_t *l) { const struct cdevsw *d; struct localcount *lc; - int rv, mpflag; + device_t dv = NULL/*XXXGCC*/; + int unit, rv, mpflag; d = cdevsw_lookup_acquire(dev, &lc); if (d == NULL) return ENXIO; + if (d->d_devtounit) { + /* + * If the device node corresponds to an autoconf device + * instance, acquire a reference to it so that during + * d_open, device_lookup is stable. + * + * XXX This should also arrange to instantiate cloning + * pseudo-devices if appropriate, but that requires + * reviewing them all to find and verify a common + * pattern. + */ + if ((unit = (*d->d_devtounit)(dev)) == -1) + return ENXIO; + if ((dv = device_lookup_acquire(d->d_cfdriver, unit)) == NULL) + return ENXIO; + } + DEV_LOCK(d); rv = (*d->d_open)(dev, flag, devtype, l); DEV_UNLOCK(d); + if (d->d_devtounit) { + device_release(dv); + } + cdevsw_release(d, lc); return rv; diff --git a/sys/sys/conf.h b/sys/sys/conf.h index ba46976a924b..12cfd3bf89a6 100644 --- a/sys/sys/conf.h +++ b/sys/sys/conf.h @@ -76,6 +76,8 @@ struct bdevsw { int (*d_dump)(dev_t, daddr_t, void *, size_t); int (*d_psize)(dev_t); int (*d_discard)(dev_t, off_t, off_t); + int (*d_devtounit)(dev_t); + struct cfdriver *d_cfdriver; int d_flag; }; @@ -94,6 +96,8 @@ struct cdevsw { paddr_t (*d_mmap)(dev_t, off_t, int); int (*d_kqfilter)(dev_t, struct knote *); int (*d_discard)(dev_t, off_t, off_t); + int (*d_devtounit)(dev_t); + struct cfdriver *d_cfdriver; int d_flag; };