diff options
| author | riastradh <riastradh@NetBSD.org> | 2023-05-22 14:58:22 +0000 |
|---|---|---|
| committer | riastradh <riastradh@NetBSD.org> | 2023-05-22 14:58:22 +0000 |
| commit | cfc732c2b4daae29c8d36a623532beac521ffe78 (patch) | |
| tree | 72533f6f2c01631ba01dbe7b47ade58b8c13d365 /sys | |
| parent | d118a3897f290a50b17a761c4656d271d68f3f4d (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.c | 189 | ||||
| -rw-r--r-- | sys/sys/device.h | 9 |
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 *); |
