summaryrefslogtreecommitdiff
path: root/sys
diff options
context:
space:
mode:
authorriastradh <riastradh@NetBSD.org>2023-05-22 14:58:22 +0000
committerriastradh <riastradh@NetBSD.org>2023-05-22 14:58:22 +0000
commitcfc732c2b4daae29c8d36a623532beac521ffe78 (patch)
tree72533f6f2c01631ba01dbe7b47ade58b8c13d365 /sys
parentd118a3897f290a50b17a761c4656d271d68f3f4d (diff)
autoconf(9): New functions for referenced attach/detach.
New functions: - config_found_acquire(dev, aux, print, cfargs) - config_attach_acquire(parent, cf, aux, print, cfargs) - config_attach_pseudo_acquire(cf, aux) - config_detach_release(dev, flags) - device_acquire(dev) The config_*_acquire functions are like the non-acquire versions, but they return a referenced device_t, which is guaranteed to be safe to use until released. The device's detach function may run while it is referenced, but the device_t will not be freed and the parent's .ca_childdetached routine will not be called. => config_attach_pseudo_acquire additionally lets you pass an aux argument to the device's .ca_attach routine, unlike config_attach_pseudo which always passes NULL. => Eventually, config_found, config_attach, and config_attach_pseudo should be made to return void, because use of the device_t they return is unsafe without the kernel lock and difficult to use safely even with the kernel lock or in a UP system. For now, they require the caller to hold the kernel lock, while config_*_acquire do not. config_detach_release is like device_release and then config_detach, but avoids the race inherent with that sequence. => Eventually, config_detach should be eliminated, because getting at the device_t it needs is unsafe without the kernel lock and difficult to use safely even with the kernel lock or in a UP system. For now, it requires the caller to hold the kernel lock, while config_detach_release does not. device_acquire acquires a reference to a device. It never fails and can be used in thread context (but not interrupt context, hard or soft). Caller is responsible for ensuring that the device_t cannot be freed; in other words, the device_t must be made unavailable to any device_acquire callers before the .ca_detach function returns. Typically device_acquire will be used in a read section (mutex, rwlock, pserialize, &c.) in a data structure lookup, with corresponding logic in the .ca_detach function to remove the device from the data structure and wait for all read sections to complete. Proposed on tech-kern: https://mail-index.netbsd.org/tech-kern/2023/05/10/msg028889.html
Diffstat (limited to 'sys')
-rw-r--r--sys/kern/subr_autoconf.c189
-rw-r--r--sys/sys/device.h9
2 files changed, 184 insertions, 14 deletions
diff --git a/sys/kern/subr_autoconf.c b/sys/kern/subr_autoconf.c
index e60be2d7aad..7d1879e8264 100644
--- a/sys/kern/subr_autoconf.c
+++ b/sys/kern/subr_autoconf.c
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_autoconf.c,v 1.310 2023/04/21 17:35:43 riastradh Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.311 2023/05/22 14:58:22 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.310 2023/04/21 17:35:43 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.311 2023/05/22 14:58:22 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -1259,17 +1259,21 @@ static const char * const msgs[] = {
* not configured, call the given `print' function and return NULL.
*/
device_t
-config_found(device_t parent, void *aux, cfprint_t print,
+config_found_acquire(device_t parent, void *aux, cfprint_t print,
const struct cfargs * const cfargs)
{
cfdata_t cf;
struct cfargs_internal store;
const struct cfargs_internal * const args =
cfargs_canonicalize(cfargs, &store);
+ device_t dev;
+
+ KERNEL_LOCK(1, NULL);
cf = config_search_internal(parent, aux, args);
if (cf != NULL) {
- return config_attach_internal(parent, cf, aux, print, args);
+ dev = config_attach_internal(parent, cf, aux, print, args);
+ goto out;
}
if (print) {
@@ -1283,7 +1287,39 @@ config_found(device_t parent, void *aux, cfprint_t print,
aprint_normal("%s", msgs[pret]);
}
- return NULL;
+ dev = NULL;
+
+out: KERNEL_UNLOCK_ONE(NULL);
+ return dev;
+}
+
+/*
+ * config_found(parent, aux, print, cfargs)
+ *
+ * Legacy entry point for callers whose use of the returned
+ * device_t is not delimited by device_release.
+ *
+ * The caller is required to hold the kernel lock as a fragile
+ * defence against races.
+ *
+ * Callers should ignore the return value or be converted to
+ * config_found_acquire with a matching device_release once they
+ * have finished with the returned device_t.
+ */
+device_t
+config_found(device_t parent, void *aux, cfprint_t print,
+ const struct cfargs * const cfargs)
+{
+ device_t dev;
+
+ KASSERT(KERNEL_LOCKED_P());
+
+ dev = config_found_acquire(parent, aux, print, cfargs);
+ if (dev == NULL)
+ return NULL;
+ device_release(dev);
+
+ return dev;
}
/*
@@ -1708,6 +1744,8 @@ config_add_attrib_dict(device_t dev)
/*
* Attach a found device.
+ *
+ * Returns the device referenced, to be released with device_release.
*/
static device_t
config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
@@ -1778,6 +1816,12 @@ config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
*/
config_pending_incr(dev);
+ /*
+ * Prevent concurrent detach from destroying the device_t until
+ * the caller has released the device.
+ */
+ device_acquire(dev);
+
/* Call the driver's attach function. */
(*dev->dv_cfattach->ca_attach)(parent, dev, aux);
@@ -1813,15 +1857,47 @@ config_attach_internal(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
}
device_t
-config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
+config_attach_acquire(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
const struct cfargs *cfargs)
{
struct cfargs_internal store;
+ device_t dev;
+
+ KERNEL_LOCK(1, NULL);
+ dev = config_attach_internal(parent, cf, aux, print,
+ cfargs_canonicalize(cfargs, &store));
+ KERNEL_UNLOCK_ONE(NULL);
+
+ return dev;
+}
+
+/*
+ * config_attach(parent, cf, aux, print, cfargs)
+ *
+ * Legacy entry point for callers whose use of the returned
+ * device_t is not delimited by device_release.
+ *
+ * The caller is required to hold the kernel lock as a fragile
+ * defence against races.
+ *
+ * Callers should ignore the return value or be converted to
+ * config_attach_acquire with a matching device_release once they
+ * have finished with the returned device_t.
+ */
+device_t
+config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
+ const struct cfargs *cfargs)
+{
+ device_t dev;
KASSERT(KERNEL_LOCKED_P());
- return config_attach_internal(parent, cf, aux, print,
- cfargs_canonicalize(cfargs, &store));
+ dev = config_attach_acquire(parent, cf, aux, print, cfargs);
+ if (dev == NULL)
+ return NULL;
+ device_release(dev);
+
+ return dev;
}
/*
@@ -1834,7 +1910,7 @@ config_attach(device_t parent, cfdata_t cf, void *aux, cfprint_t print,
* name by the attach routine.
*/
device_t
-config_attach_pseudo(cfdata_t cf)
+config_attach_pseudo_acquire(cfdata_t cf, void *aux)
{
device_t dev;
@@ -1867,8 +1943,14 @@ config_attach_pseudo(cfdata_t cf)
*/
config_pending_incr(dev);
+ /*
+ * Prevent concurrent detach from destroying the device_t until
+ * the caller has released the device.
+ */
+ device_acquire(dev);
+
/* Call the driver's attach function. */
- (*dev->dv_cfattach->ca_attach)(ROOT, dev, NULL);
+ (*dev->dv_cfattach->ca_attach)(ROOT, dev, aux);
/*
* Allow other threads to acquire references to the device now
@@ -1893,6 +1975,36 @@ out: KERNEL_UNLOCK_ONE(NULL);
}
/*
+ * config_attach_pseudo(cf)
+ *
+ * Legacy entry point for callers whose use of the returned
+ * device_t is not delimited by device_release.
+ *
+ * The caller is required to hold the kernel lock as a fragile
+ * defence against races.
+ *
+ * Callers should ignore the return value or be converted to
+ * config_attach_pseudo_acquire with a matching device_release
+ * once they have finished with the returned device_t. As a
+ * bonus, config_attach_pseudo_acquire can pass a non-null aux
+ * argument into the driver's attach routine.
+ */
+device_t
+config_attach_pseudo(cfdata_t cf)
+{
+ device_t dev;
+
+ KASSERT(KERNEL_LOCKED_P());
+
+ dev = config_attach_pseudo_acquire(cf, NULL);
+ if (dev == NULL)
+ return dev;
+ device_release(dev);
+
+ return dev;
+}
+
+/*
* Caller must hold alldevs_lock.
*/
static void
@@ -2000,9 +2112,12 @@ config_detach_exit(device_t dev)
* Note that this code wants to be run from a process context, so
* that the detach can sleep to allow processes which have a device
* open to run and unwind their stacks.
+ *
+ * Caller must hold a reference with device_acquire or
+ * device_lookup_acquire.
*/
int
-config_detach(device_t dev, int flags)
+config_detach_release(device_t dev, int flags)
{
struct alldevs_foray af;
struct cftable *ct;
@@ -2031,6 +2146,7 @@ config_detach(device_t dev, int flags)
* attached.
*/
rv = config_detach_enter(dev);
+ device_release(dev);
if (rv) {
KERNEL_UNLOCK_ONE(NULL);
return rv;
@@ -2186,6 +2302,32 @@ out:
}
/*
+ * config_detach(dev, flags)
+ *
+ * Legacy entry point for callers that have not acquired a
+ * reference to dev.
+ *
+ * The caller is required to hold the kernel lock as a fragile
+ * defence against races.
+ *
+ * Callers should be converted to use device_acquire under a lock
+ * taken also by .ca_childdetached to synchronize access to the
+ * device_t, and then config_detach_release ouside the lock.
+ * Alternatively, most drivers detach children only in their own
+ * detach routines, which can be done with config_detach_children
+ * instead.
+ */
+int
+config_detach(device_t dev, int flags)
+{
+
+ KASSERT(KERNEL_LOCKED_P());
+
+ device_acquire(dev);
+ return config_detach_release(dev, flags);
+}
+
+/*
* config_detach_commit(dev)
*
* Issued by a driver's .ca_detach routine to notify anyone
@@ -2740,7 +2882,7 @@ retry: if (unit < 0 || unit >= cd->cd_ndevs ||
mutex_enter(&alldevs_lock);
goto retry;
}
- localcount_acquire(dv->dv_localcount);
+ device_acquire(dv);
}
mutex_exit(&alldevs_lock);
mutex_exit(&config_misc_lock);
@@ -2749,9 +2891,30 @@ retry: if (unit < 0 || unit >= cd->cd_ndevs ||
}
/*
+ * device_acquire:
+ *
+ * Acquire a reference to a device. It is the caller's
+ * responsibility to ensure that the device's .ca_detach routine
+ * cannot return before calling this. Caller must release the
+ * reference with device_release or config_detach_release.
+ */
+void
+device_acquire(device_t dv)
+{
+
+ /*
+ * No lock because the caller has promised that this can't
+ * change concurrently with device_acquire.
+ */
+ KASSERTMSG(!dv->dv_detach_done, "%s",
+ dv == NULL ? "(null)" : device_xname(dv));
+ localcount_acquire(dv->dv_localcount);
+}
+
+/*
* device_release:
*
- * Release a reference to a device acquired with
+ * Release a reference to a device acquired with device_acquire or
* device_lookup_acquire.
*/
void
diff --git a/sys/sys/device.h b/sys/sys/device.h
index 4f47e063d0a..2c67b99b028 100644
--- a/sys/sys/device.h
+++ b/sys/sys/device.h
@@ -1,4 +1,4 @@
-/* $NetBSD: device.h,v 1.185 2022/08/24 11:19:25 riastradh Exp $ */
+/* $NetBSD: device.h,v 1.186 2023/05/22 14:58:22 riastradh Exp $ */
/*
* Copyright (c) 2021 The NetBSD Foundation, Inc.
@@ -554,14 +554,20 @@ device_t config_found(device_t, void *, cfprint_t, const struct cfargs *);
device_t config_rootfound(const char *, void *);
device_t config_attach(device_t, cfdata_t, void *, cfprint_t,
const struct cfargs *);
+device_t config_found_acquire(device_t, void *, cfprint_t,
+ const struct cfargs *);
+device_t config_attach_acquire(device_t, cfdata_t, void *, cfprint_t,
+ const struct cfargs *);
int config_match(device_t, cfdata_t, void *);
int config_probe(device_t, cfdata_t, void *);
bool ifattr_match(const char *, const char *);
device_t config_attach_pseudo(cfdata_t);
+device_t config_attach_pseudo_acquire(cfdata_t, void *);
int config_detach(device_t, int);
+int config_detach_release(device_t, int);
int config_detach_children(device_t, int flags);
void config_detach_commit(device_t);
bool config_detach_all(int);
@@ -588,6 +594,7 @@ device_t device_lookup(cfdriver_t, int);
void *device_lookup_private(cfdriver_t, int);
device_t device_lookup_acquire(cfdriver_t, int);
+void device_acquire(device_t);
void device_release(device_t);
void device_register(device_t, void *);