[src/trunk]: src/sys/dev Remove unnecessary wait in ldbegindetach.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/854d0323eb5a
branches:  trunk
changeset: 936698:854d0323eb5a
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Aug 02 01:17:56 2020 +0000

description:
Remove unnecessary wait in ldbegindetach.

Like disk_begindetach, ldbegindetach only commits to detaching but
doesn't wait for existing xfers to drain; it is up to the driver to
abort them, once we are committed, and then ldenddetach to wait for
them to drain.

diffstat:

 sys/dev/ld.c             |  53 ++++++++++++++++++++++++-----------------------
 sys/dev/ldvar.h          |   4 +-
 sys/dev/sdmmc/ld_sdmmc.c |  28 +++++++++++++-----------
 3 files changed, 44 insertions(+), 41 deletions(-)

diffs (170 lines):

diff -r bf40c236c7b0 -r 854d0323eb5a sys/dev/ld.c
--- a/sys/dev/ld.c      Sun Aug 02 00:20:21 2020 +0000
+++ b/sys/dev/ld.c      Sun Aug 02 01:17:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ld.c,v 1.110 2020/04/13 08:05:02 maxv Exp $    */
+/*     $NetBSD: ld.c,v 1.111 2020/08/02 01:17:56 riastradh Exp $       */
 
 /*-
  * Copyright (c) 1998, 2000 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ld.c,v 1.110 2020/04/13 08:05:02 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ld.c,v 1.111 2020/08/02 01:17:56 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -189,26 +189,25 @@
 ldbegindetach(struct ld_softc *sc, int flags)
 {
        struct dk_softc *dksc = &sc->sc_dksc;
-       int rv = 0;
-
-       if ((sc->sc_flags & LDF_ENABLED) == 0)
-               return (0);
+       int error;
 
-       rv = disk_begindetach(&dksc->sc_dkdev, ld_lastclose, dksc->sc_dev, flags);
-
-       if (rv != 0)
-               return rv;
+       /* If we never attached properly, no problem with detaching.  */
+       if ((sc->sc_flags & LDF_ENABLED) == 0)
+               return 0;
 
-       mutex_enter(&sc->sc_mutex);
-       sc->sc_maxqueuecnt = 0;
+       /*
+        * If the disk is still open, back out before we commit to
+        * detaching.
+        */
+       error = disk_begindetach(&dksc->sc_dkdev, ld_lastclose, dksc->sc_dev,
+           flags);
+       if (error)
+               return error;
 
-       while (sc->sc_queuecnt > 0) {
-               sc->sc_flags |= LDF_DRAIN;
-               cv_wait(&sc->sc_drain, &sc->sc_mutex);
-       }
-       mutex_exit(&sc->sc_mutex);
+       /* We are now committed to detaching.  Prevent new xfers.  */
+       ldadjqparam(sc, 0);
 
-       return (rv);
+       return 0;
 }
 
 void
@@ -220,12 +219,17 @@
        if ((sc->sc_flags & LDF_ENABLED) == 0)
                return;
 
+       /* Wait for commands queued with the hardware to complete. */
        mutex_enter(&sc->sc_mutex);
-
-       /* Wait for commands queued with the hardware to complete. */
-       if (sc->sc_queuecnt != 0) {
-               if (cv_timedwait(&sc->sc_drain, &sc->sc_mutex, 30 * hz))
+       while (sc->sc_queuecnt > 0) {
+               if (cv_timedwait(&sc->sc_drain, &sc->sc_mutex, 30 * hz)) {
+                       /*
+                        * XXX This seems like a recipe for crashing on
+                        * use after free...
+                        */
                        printf("%s: not drained\n", dksc->sc_xname);
+                       break;
+               }
        }
        mutex_exit(&sc->sc_mutex);
 
@@ -467,10 +471,7 @@
 
        mutex_enter(&sc->sc_mutex);
        if (--sc->sc_queuecnt <= sc->sc_maxqueuecnt) {
-               if ((sc->sc_flags & LDF_DRAIN) != 0) {
-                       sc->sc_flags &= ~LDF_DRAIN;
-                       cv_broadcast(&sc->sc_drain);
-               }
+               cv_broadcast(&sc->sc_drain);
                mutex_exit(&sc->sc_mutex);
                dk_start(dksc, NULL);
        } else
diff -r bf40c236c7b0 -r 854d0323eb5a sys/dev/ldvar.h
--- a/sys/dev/ldvar.h   Sun Aug 02 00:20:21 2020 +0000
+++ b/sys/dev/ldvar.h   Sun Aug 02 01:17:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ldvar.h,v 1.33 2019/03/19 07:01:14 mlelstv Exp $       */
+/*     $NetBSD: ldvar.h,v 1.34 2020/08/02 01:17:56 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2000 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
 
 /* sc_flags */
 #define        LDF_ENABLED     0x001           /* device enabled */
-#define        LDF_DRAIN       0x020           /* maxqueuecnt has changed; drain */
+#define        LDF_UNUSED0     0x020           /* was LDF_DRAIN */
 #define        LDF_NO_RND      0x040           /* do not attach rnd source */
 #define        LDF_MPSAFE      0x080           /* backend is MPSAFE */
 
diff -r bf40c236c7b0 -r 854d0323eb5a sys/dev/sdmmc/ld_sdmmc.c
--- a/sys/dev/sdmmc/ld_sdmmc.c  Sun Aug 02 00:20:21 2020 +0000
+++ b/sys/dev/sdmmc/ld_sdmmc.c  Sun Aug 02 01:17:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ld_sdmmc.c,v 1.40 2020/07/22 17:18:10 riastradh Exp $  */
+/*     $NetBSD: ld_sdmmc.c,v 1.41 2020/08/02 01:17:56 riastradh Exp $  */
 
 /*
  * Copyright (c) 2008 KIYOHARA Takashi
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ld_sdmmc.c,v 1.40 2020/07/22 17:18:10 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ld_sdmmc.c,v 1.41 2020/08/02 01:17:56 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_sdmmc.h"
@@ -344,11 +344,20 @@
        struct ld_sdmmc_softc *sc = device_private(dev);
        struct ld_softc *ld = &sc->sc_ld;
        struct ld_sdmmc_task *task;
-       int rv, i;
+       int error, i;
 
        /*
-        * Block new xfers, abort all pending tasks, and wait for all
-        * pending waiters to notice that we're gone.
+        * Block new xfers, or fail if the disk is still open and the
+        * detach isn't forced.  After this point, we are committed to
+        * detaching.
+        */
+       error = ldbegindetach(ld, flags);
+       if (error)
+               return error;
+
+       /*
+        * Abort all pending tasks, and wait for all pending waiters to
+        * notice that we're gone.
         */
        mutex_enter(&sc->sc_lock);
        sc->sc_dying = true;
@@ -358,14 +367,7 @@
                cv_wait(&sc->sc_cv, &sc->sc_lock);
        mutex_exit(&sc->sc_lock);
 
-       /* Do the ld detach dance.  */
-       if ((rv = ldbegindetach(ld, flags)) != 0) {
-               /* Detach failed -- back out.  */
-               mutex_enter(&sc->sc_lock);
-               sc->sc_dying = false;
-               mutex_exit(&sc->sc_lock);
-               return rv;
-       }
+       /* Done!  Destroy the disk.  */
        ldenddetach(ld);
 
        KASSERT(TAILQ_EMPTY(&sc->sc_xferq));