git: 25c49c426c6b - main - Revert "Make device_busy/unbusy work w/o Giant held"

From: Warner Losh <imp_at_FreeBSD.org>
Date: Tue, 30 Nov 2021 22:21:03 UTC
The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=25c49c426c6b6067f7374fae39fb38333cd11e0c

commit 25c49c426c6b6067f7374fae39fb38333cd11e0c
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2021-11-30 22:12:22 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-11-30 22:17:07 +0000

    Revert "Make device_busy/unbusy work w/o Giant held"
    
    This reverts commit 08e781915363f98f4318a864b3b5a52bd99424c6.
    
    Commit message was for a very old version of the patch. Will re-commit
    with the right one since it's so bad. There's no locked versions of
    it...that code was reworked to use refcnt APIs.
    
    Noticed by:     jhb, jtrc27
    Sponsored by:   Netflix
---
 sys/dev/drm2/drm_fops.c |  6 ++++++
 sys/dev/gpio/gpiopps.c  |  9 +++++++--
 sys/dev/pccard/pccard.c |  2 +-
 sys/kern/subr_bus.c     | 41 ++++++++++++++++++++++++-----------------
 sys/sys/bus.h           |  1 +
 5 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/sys/dev/drm2/drm_fops.c b/sys/dev/drm2/drm_fops.c
index 0c8b71759292..3b3be06345e5 100644
--- a/sys/dev/drm2/drm_fops.c
+++ b/sys/dev/drm2/drm_fops.c
@@ -157,7 +157,9 @@ int drm_open(struct cdev *kdev, int flags, int fmt, DRM_STRUCTPROC *p)
 	return 0;
 
 err_undo:
+	mtx_lock(&Giant); /* FIXME: Giant required? */
 	device_unbusy(dev->dev);
+	mtx_unlock(&Giant);
 	dev->open_count--;
 	sx_xunlock(&drm_global_mutex);
 	return -retcode;
@@ -271,7 +273,9 @@ static int drm_open_helper(struct cdev *kdev, int flags, int fmt,
 	list_add(&priv->lhead, &dev->filelist);
 	DRM_UNLOCK(dev);
 
+	mtx_lock(&Giant); /* FIXME: Giant required? */
 	device_busy(dev->dev);
+	mtx_unlock(&Giant);
 
 	ret = devfs_set_cdevpriv(priv, drm_release);
 	if (ret != 0)
@@ -449,7 +453,9 @@ void drm_release(void *data)
 	 */
 
 	atomic_inc(&dev->counts[_DRM_STAT_CLOSES]);
+	mtx_lock(&Giant);
 	device_unbusy(dev->dev);
+	mtx_unlock(&Giant);
 	if (!--dev->open_count) {
 		if (atomic_read(&dev->ioctl_count)) {
 			DRM_ERROR("Device busy: %d\n",
diff --git a/sys/dev/gpio/gpiopps.c b/sys/dev/gpio/gpiopps.c
index 741bfa4498a6..4700acf19bcd 100644
--- a/sys/dev/gpio/gpiopps.c
+++ b/sys/dev/gpio/gpiopps.c
@@ -73,7 +73,9 @@ gpiopps_open(struct cdev *dev, int flags, int fmt, struct thread *td)
 
 	/* We can't be unloaded while open, so mark ourselves BUSY. */
 	mtx_lock(&sc->pps_mtx);
-	device_busy(sc->dev);
+	if (device_get_state(sc->dev) < DS_BUSY) {
+		device_busy(sc->dev);
+	}
 	mtx_unlock(&sc->pps_mtx);
 
 	return 0;
@@ -84,6 +86,10 @@ gpiopps_close(struct cdev *dev, int flags, int fmt, struct thread *td)
 {
 	struct pps_softc *sc = dev->si_drv1;
 
+	/*
+	 * Un-busy on last close. We rely on the vfs counting stuff to only call
+	 * this routine on last-close, so we don't need any open-count logic.
+	 */
 	mtx_lock(&sc->pps_mtx);
 	device_unbusy(sc->dev);
 	mtx_unlock(&sc->pps_mtx);
@@ -107,7 +113,6 @@ gpiopps_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flags, struct thre
 
 static struct cdevsw pps_cdevsw = {
 	.d_version =    D_VERSION,
-	.d_flags =	D_TRACKCLOSE,
 	.d_open =       gpiopps_open,
 	.d_close =      gpiopps_close,
 	.d_ioctl =      gpiopps_ioctl,
diff --git a/sys/dev/pccard/pccard.c b/sys/dev/pccard/pccard.c
index 48d2e6973a38..8f19eb84725c 100644
--- a/sys/dev/pccard/pccard.c
+++ b/sys/dev/pccard/pccard.c
@@ -342,7 +342,7 @@ pccard_detach_card(device_t dev)
 		if (pf->dev == NULL)
 			continue;
 		state = device_get_state(pf->dev);
-		if (state == DS_ATTACHED)
+		if (state == DS_ATTACHED || state == DS_BUSY)
 			device_detach(pf->dev);
 		if (pf->cfe != NULL)
 			pccard_function_disable(pf);
diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index ab7de881d57d..09072e1a23de 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -51,7 +51,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/queue.h>
 #include <machine/bus.h>
 #include <sys/random.h>
-#include <sys/refcount.h>
 #include <sys/rman.h>
 #include <sys/sbuf.h>
 #include <sys/selinfo.h>
@@ -141,7 +140,7 @@ struct _device {
 	int		unit;		/**< current unit number */
 	char*		nameunit;	/**< name+unit e.g. foodev0 */
 	char*		desc;		/**< driver specific description */
-	u_int		busy;		/**< count of calls to device_busy() */
+	int		busy;		/**< count of calls to device_busy() */
 	device_state_t	state;		/**< current device state  */
 	uint32_t	devflags;	/**< api level flags for device_get_flags() */
 	u_int		flags;		/**< internal device flags  */
@@ -2635,13 +2634,13 @@ device_disable(device_t dev)
 void
 device_busy(device_t dev)
 {
-
-	/*
-	 * Mark the device as busy, recursively up the tree if this busy count
-	 * goes 0->1.
-	 */
-	if (refcount_acquire(&dev->busy) == 0 && dev->parent != NULL)
+	if (dev->state < DS_ATTACHING)
+		panic("device_busy: called for unattached device");
+	if (dev->busy == 0 && dev->parent)
 		device_busy(dev->parent);
+	dev->busy++;
+	if (dev->state == DS_ATTACHED)
+		dev->state = DS_BUSY;
 }
 
 /**
@@ -2650,12 +2649,17 @@ device_busy(device_t dev)
 void
 device_unbusy(device_t dev)
 {
-
-	/*
-	 * Mark the device as unbsy, recursively if this is the last busy count.
-	 */
-	if (refcount_release(&dev->busy) && dev->parent != NULL)
-		device_unbusy(dev->parent);
+	if (dev->busy != 0 && dev->state != DS_BUSY &&
+	    dev->state != DS_ATTACHING)
+		panic("device_unbusy: called for non-busy device %s",
+		    device_get_nameunit(dev));
+	dev->busy--;
+	if (dev->busy == 0) {
+		if (dev->parent)
+			device_unbusy(dev->parent);
+		if (dev->state == DS_BUSY)
+			dev->state = DS_ATTACHED;
+	}
 }
 
 /**
@@ -3000,7 +3004,10 @@ device_attach(device_t dev)
 	attachentropy = (uint16_t)(get_cyclecount() - attachtime);
 	random_harvest_direct(&attachentropy, sizeof(attachentropy), RANDOM_ATTACH);
 	device_sysctl_update(dev);
-	dev->state = DS_ATTACHED;
+	if (dev->busy)
+		dev->state = DS_BUSY;
+	else
+		dev->state = DS_ATTACHED;
 	dev->flags &= ~DF_DONENOMATCH;
 	EVENTHANDLER_DIRECT_INVOKE(device_attach, dev);
 	devadded(dev);
@@ -3031,7 +3038,7 @@ device_detach(device_t dev)
 	GIANT_REQUIRED;
 
 	PDEBUG(("%s", DEVICENAME(dev)));
-	if (dev->busy > 0)
+	if (dev->state == DS_BUSY)
 		return (EBUSY);
 	if (dev->state == DS_ATTACHING) {
 		device_printf(dev, "device in attaching state! Deferring detach.\n");
@@ -3083,7 +3090,7 @@ int
 device_quiesce(device_t dev)
 {
 	PDEBUG(("%s", DEVICENAME(dev)));
-	if (dev->busy > 0)
+	if (dev->state == DS_BUSY)
 		return (EBUSY);
 	if (dev->state != DS_ATTACHED)
 		return (0);
diff --git a/sys/sys/bus.h b/sys/sys/bus.h
index 4a1afa864c2f..0942b7246ae4 100644
--- a/sys/sys/bus.h
+++ b/sys/sys/bus.h
@@ -58,6 +58,7 @@ typedef enum device_state {
 	DS_ALIVE = 20,			/**< @brief probe succeeded */
 	DS_ATTACHING = 25,		/**< @brief currently attaching */
 	DS_ATTACHED = 30,		/**< @brief attach method called */
+	DS_BUSY = 40			/**< @brief device is open */
 } device_state_t;
 
 /**