From 3fdb699668b8ba613eee15a2e6571a01039389d0 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Tue, 1 Feb 2022 12:57:16 +0000 Subject: [PATCH 05/13] autoconf(9): Fast paths for device_lookup and device_lookup_acquire. --- sys/kern/subr_autoconf.c | 92 +++++++++++++++++++++++++++++++++------- 1 file changed, 77 insertions(+), 15 deletions(-) diff --git a/sys/kern/subr_autoconf.c b/sys/kern/subr_autoconf.c index 0a87bc954838..3223494bd0e5 100644 --- a/sys/kern/subr_autoconf.c +++ b/sys/kern/subr_autoconf.c @@ -109,6 +109,9 @@ __KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.291 2021/12/31 14:19:57 riastrad #include #include #include +#include +#include +#include #include @@ -1364,8 +1367,16 @@ config_makeroom(int n, struct cfdriver *cd) if (ondevs != 0) memcpy(nsp, cd->cd_devs, sizeof(device_t) * ondevs); - cd->cd_ndevs = nndevs; - cd->cd_devs = nsp; + /* + * Publish the new array first, then publish the new + * length, so pserialized readers reading the length + * first will always see it at most as long as is valid + * to access. + */ + KASSERT(cd->cd_ndevs <= nndevs); + atomic_store_release(&cd->cd_devs, nsp); + atomic_store_release(&cd->cd_ndevs, nndevs); + if (ondevs != 0) { mutex_exit(&alldevs_lock); kmem_free(osp, sizeof(device_t) * ondevs); @@ -1425,7 +1436,7 @@ config_devunlink(device_t dev, struct devicelist *garbage) TAILQ_INSERT_TAIL(garbage, dev, dv_list); /* Remove from cfdriver's array. */ - cd->cd_devs[dev->dv_unit] = NULL; + atomic_store_relaxed(&cd->cd_devs[dev->dv_unit], NULL); /* * If the device now has no units in use, unlink its softc array. @@ -1438,8 +1449,8 @@ config_devunlink(device_t dev, struct devicelist *garbage) if (i == cd->cd_ndevs) { dg->dg_ndevs = cd->cd_ndevs; dg->dg_devs = cd->cd_devs; - cd->cd_devs = NULL; - cd->cd_ndevs = 0; + atomic_store_relaxed(&cd->cd_devs, NULL); + atomic_store_relaxed(&cd->cd_ndevs, 0); } } @@ -1507,8 +1518,8 @@ config_unit_alloc(device_t dev, cfdriver_t cd, cfdata_t cf) if (unit == -1) break; if (unit < cd->cd_ndevs) { - cd->cd_devs[unit] = dev; dev->dv_unit = unit; + atomic_store_release(&cd->cd_devs[unit], dev); break; } config_makeroom(unit, cd); @@ -1914,6 +1925,15 @@ config_dump_garbage(struct devicelist *garbage) { device_t dv; + if (TAILQ_EMPTY(garbage)) + return; + + /* + * Wait for all pserialize readers to notice that devices have + * been deleted before we free them. + */ + xc_barrier(0); + while ((dv = TAILQ_FIRST(garbage)) != NULL) { TAILQ_REMOVE(garbage, dv, dv_list); config_devdelete(dv); @@ -2178,7 +2198,7 @@ config_detach_commit(device_t dev) mutex_enter(&config_misc_lock); KASSERT(dev->dv_detaching == curlwp); - dev->dv_detached = true; + atomic_store_relaxed(&dev->dv_detached, true); cv_broadcast(&config_misc_cv); mutex_exit(&config_misc_lock); } @@ -2638,14 +2658,34 @@ config_alldevs_exit(struct alldevs_foray *af) device_t device_lookup(cfdriver_t cd, int unit) { - device_t dv; + device_t dv = NULL, *devs; + int s; - mutex_enter(&alldevs_lock); - if (unit < 0 || unit >= cd->cd_ndevs) - dv = NULL; - else if ((dv = cd->cd_devs[unit]) != NULL && dv->detached) + s = pserialize_read_enter(); + /* + * We can safely use atomic_load_relaxed here, instead of the + * costlier atomic_load_acquire, because the caller is + * obligated to ensure the device is stable. We still need to + * use pserialize and atomic_load_consume in case the array of + * devices is being updated, so that if we see the old array, + * it doesn't go away before we're done, and if we see the new + * array, we also see its initialized content. + * + * XXX But let's leave it as atomic_load_acquire until we've + * distributed .d_cfdriver/devtounit to all devsw instances + * that might need it, particularly removables like USB. + */ + if (unit < 0 || unit >= atomic_load_acquire(&cd->cd_ndevs)) + goto out; + if ((devs = atomic_load_consume(&cd->cd_devs)) == NULL) + goto out; /* raced with config_devunlink */ + if ((dv = atomic_load_consume(&devs[unit])) == NULL) + goto out; + if (atomic_load_relaxed(&dv->dv_detached) != 0) { dv = NULL; - mutex_exit(&alldevs_lock); + goto out; + } +out: pserialize_read_exit(s); return dv; } @@ -2681,11 +2721,33 @@ device_lookup_private(cfdriver_t cd, int unit) device_t device_lookup_acquire(cfdriver_t cd, int unit) { - device_t dv; + device_t dv = NULL, *devs; + int s; ASSERT_SLEEPABLE(); - /* XXX This should have a pserialized fast path -- TBD. */ + s = pserialize_read_enter(); + if (unit < 0 || unit >= atomic_load_acquire(&cd->cd_ndevs)) + goto out; + if ((devs = atomic_load_consume(&cd->cd_devs)) == NULL) + goto out; /* raced with config_devunlink */ + if ((dv = atomic_load_consume(&devs[unit])) == NULL) + goto out; + if (__predict_false(atomic_load_relaxed(&dv->dv_detached))) { + dv = NULL; + goto out; + } + if (__predict_false(dv->dv_attaching != NULL) && + dv->dv_attaching != curlwp) + goto slow; + if (__predict_false(dv->dv_detaching != NULL)) + goto slow; + localcount_acquire(dv->dv_localcount); +out: pserialize_read_exit(s); + return dv; + +slow: dv = NULL; + pserialize_read_exit(s); mutex_enter(&config_misc_lock); mutex_enter(&alldevs_lock); retry: if (unit < 0 || unit >= cd->cd_ndevs ||