Module Name: src
Committed By: riastradh
Date: Tue Sep 13 09:43:33 UTC 2022
Modified Files:
src/sys/kern: subr_autoconf.c
src/sys/sys: device_impl.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.305 -r1.306 src/sys/kern/subr_autoconf.c
cvs rdiff -u -r1.4 -r1.5 src/sys/sys/device_impl.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/subr_autoconf.c
diff -u src/sys/kern/subr_autoconf.c:1.305 src/sys/kern/subr_autoconf.c:1.306
--- src/sys/kern/subr_autoconf.c:1.305 Tue Sep 13 09:40:38 2022
+++ src/sys/kern/subr_autoconf.c Tue Sep 13 09:43:33 2022
@@ -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 @@ config_detach(device_t dev, int flags)
} 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 @@ config_detach(device_t dev, int flags)
* 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 @@ config_detach(device_t dev, int flags)
/*
* 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 @@ config_detach_commit(device_t dev)
"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 @@ device_lookup_acquire(cfdriver_t cd, int
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 {
/*
Index: src/sys/sys/device_impl.h
diff -u src/sys/sys/device_impl.h:1.4 src/sys/sys/device_impl.h:1.5
--- src/sys/sys/device_impl.h:1.4 Wed Aug 24 11:47:52 2022
+++ src/sys/sys/device_impl.h Tue Sep 13 09:43:33 2022
@@ -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 device {
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);
