Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys autoconf(9): New diagnostic to detect double-detach.



details:   https://anonhg.NetBSD.org/src/rev/fae356bd1663
branches:  trunk
changeset: 370043:fae356bd1663
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Sep 13 09:43:33 2022 +0000

description:
autoconf(9): New diagnostic to detect double-detach.

- Rename dv_detached -> dv_detach_committed.
- Add dv_detach_done, asserted false and then set in config_detach.

dv_detach_done may appear redundant with dv_del_gen, but dv_del_gen
will be used to safely detect config_detach on two valid references
to a device (e.g., a bus detaching its child concurrently with drvctl
detaching the same child), while dv_detach_done is strictly a
diagnostic to detect races in the config_detach API.

Currently the config_detach API itself is unsafe, but we can add a
config_detach_release function that simultaneously releases and
detaches a referenced device_t; this will continue to use dv_del_gen
to safely avoid multiple detach, and dv_detach_done to check for
races in usage.

diffstat:

 sys/kern/subr_autoconf.c |  18 +++++++++++-------
 sys/sys/device_impl.h    |   5 +++--
 2 files changed, 14 insertions(+), 9 deletions(-)

diffs (86 lines):

diff -r e7521f6b4968 -r fae356bd1663 sys/kern/subr_autoconf.c
--- a/sys/kern/subr_autoconf.c  Tue Sep 13 09:40:38 2022 +0000
+++ b/sys/kern/subr_autoconf.c  Tue Sep 13 09:43:33 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_autoconf.c,v 1.305 2022/09/13 09:40:38 riastradh Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.306 2022/09/13 09:43:33 riastradh Exp $ */
 
 /*
  * Copyright (c) 1996, 2000 Christopher G. Demetriou
@@ -77,7 +77,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.305 2022/09/13 09:40:38 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.306 2022/09/13 09:43:33 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -2051,6 +2051,9 @@
        } else
                rv = EOPNOTSUPP;
 
+       KASSERTMSG(!dev->dv_detach_done, "%s detached twice, error=%d",
+           device_xname(dev), rv);
+
        /*
         * If it was not possible to detach the device, then we either
         * panic() (for the forced but failed case), or return an error.
@@ -2060,9 +2063,9 @@
                 * Detach failed -- likely EOPNOTSUPP or EBUSY.  Driver
                 * must not have called config_detach_commit.
                 */
-               KASSERTMSG(!dev->dv_detached,
-                   "%s committed to detaching and then backed out",
-                   device_xname(dev));
+               KASSERTMSG(!dev->dv_detach_committed,
+                   "%s committed to detaching and then backed out, error=%d",
+                   device_xname(dev), rv);
                if (flags & DETACH_FORCE) {
                        panic("config_detach: forced detach of %s failed (%d)",
                            device_xname(dev), rv);
@@ -2073,6 +2076,7 @@
        /*
         * The device has now been successfully detached.
         */
+       dev->dv_detach_done = true;
 
        /*
         * If .ca_detach didn't commit to detach, then do that for it.
@@ -2193,7 +2197,7 @@
            "lwp %ld [%s] @ %p detaching %s",
            (long)l->l_lid, (l->l_name ? l->l_name : l->l_proc->p_comm), l,
            device_xname(dev));
-       dev->dv_detached = true;
+       dev->dv_detach_committed = true;
        cv_broadcast(&config_misc_cv);
        mutex_exit(&config_misc_lock);
 }
@@ -2706,7 +2710,7 @@
 retry: if (unit < 0 || unit >= cd->cd_ndevs ||
            (dv = cd->cd_devs[unit]) == NULL ||
            dv->dv_del_gen != 0 ||
-           dv->dv_detached) {
+           dv->dv_detach_committed) {
                dv = NULL;
        } else {
                /*
diff -r e7521f6b4968 -r fae356bd1663 sys/sys/device_impl.h
--- a/sys/sys/device_impl.h     Tue Sep 13 09:40:38 2022 +0000
+++ b/sys/sys/device_impl.h     Tue Sep 13 09:43:33 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: device_impl.h,v 1.4 2022/08/24 11:47:52 riastradh Exp $        */
+/*     $NetBSD: device_impl.h,v 1.5 2022/09/13 09:43:33 riastradh Exp $        */
 
 /*
  * Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -147,7 +147,8 @@
 
        struct lwp      *dv_attaching;  /* thread not yet finished in attach */
        struct lwp      *dv_detaching;  /* detach lock (config_misc_lock/cv) */
-       bool            dv_detached;    /* config_misc_lock */
+       bool            dv_detach_committed;    /* config_misc_lock */
+       bool            dv_detach_done;         /* dv_detaching */
 
        size_t          dv_activity_count;
        void            (**dv_activity_handlers)(device_t, devactive_t);



Home | Main Index | Thread Index | Old Index