From nobody Tue Nov 30 22:21:03 2021 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id D936F18A27DF; Tue, 30 Nov 2021 22:21:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J3cB749Hsz4SVY; Tue, 30 Nov 2021 22:21:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 6F9C31E4CF; Tue, 30 Nov 2021 22:21:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1AUML3Xo017837; Tue, 30 Nov 2021 22:21:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1AUML3xS017836; Tue, 30 Nov 2021 22:21:03 GMT (envelope-from git) Date: Tue, 30 Nov 2021 22:21:03 GMT Message-Id: <202111302221.1AUML3xS017836@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Warner Losh Subject: git: 25c49c426c6b - main - Revert "Make device_busy/unbusy work w/o Giant held" List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 25c49c426c6b6067f7374fae39fb38333cd11e0c Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638310863; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bd36ILyr5EbAqOoj9u04Au2BzC+jtNzC8O8NUlcbs8s=; b=r4sdCDywY78iNWhyW/ai7efVW7QqKqdVCWInRnNgPxObo8LkzqCl7N2FEYt6XbW+OUEpac EB5IT6VaufS3USYwhILd6dRTmcIKK/FGoZBlfvqCfw/wXTGMgj8vBxQfLRo+0c4pgpc31X 71K0B7emh/HX3dA+imCBhuUvrOtjedBVWgYQ8eJ7Dj30/EMcS9nLqU4Mvcs5bSMvbrUDm7 LL4aUuPNZFxHICdVJnqymjSyXB6m+GbJI7VZrW1NJQTUeLCaRhXc3egNTPzAKtvQGHeapA uFcg/2RZvIJFCq6zOvMpxFxreQ3xjHURrsQpKozVjJO9NSEYUHrC9tLBDQ2i+w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638310863; a=rsa-sha256; cv=none; b=EnMrm4vWRvmrd8wodJheryHXEM6a+Qtp7G/C0q+eIdM9i3F6uvoEcRtA1V5KzIKGDMn3n7 okOOWjfhxtPaeashDpPCqwKWwzdAiYH1OJqRZHkIrVU4AUCcc492os5OPJVcjVU4G4liSz 90z5F50bh7qpZQyJZlfIRh6dgAyHNfFTXXJqljVpZ/Htk6gQ/dTEDPUc/6H0WpIXE/WB61 xQqMV/uoeaoNI8irZjB557snPQ7ZdGwFmKP4ILlHMPQ+Cqzz25qD8jiznBDhlJ0BWd2tb2 TbrOIwN67Q+NttAhReqT/v/0TDteB8VXlUV1F/m53eg2cI10wKqeB80/am1khA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=25c49c426c6b6067f7374fae39fb38333cd11e0c commit 25c49c426c6b6067f7374fae39fb38333cd11e0c Author: Warner Losh AuthorDate: 2021-11-30 22:12:22 +0000 Commit: Warner Losh 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 #include #include -#include #include #include #include @@ -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; /**