Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src-draft/trunk]: src/sys/dev/sdmmc Fix races in sdmmc tasks and teach ld@sd...
details: https://anonhg.NetBSD.org/src-all/rev/02624e39f278
branches: trunk
changeset: 933203:02624e39f278
user: Taylor R Campbell <riastradh%NetBSD.org@localhost>
date: Fri May 22 02:44:03 2020 +0000
description:
Fix races in sdmmc tasks and teach ld@sdmmc to abort xfers on detach.
diffstat:
sys/dev/sdmmc/if_bwfm_sdio.c | 42 +------
sys/dev/sdmmc/ld_sdmmc.c | 215 +++++++++++++++++++++++++++++++++---------
sys/dev/sdmmc/sdmmc.c | 53 ++++++++--
sys/dev/sdmmc/sdmmc_io.c | 5 +-
sys/dev/sdmmc/sdmmcvar.h | 6 +-
5 files changed, 217 insertions(+), 104 deletions(-)
diffs (truncated from 592 to 300 lines):
diff -r 21bda4d546c9 -r 02624e39f278 sys/dev/sdmmc/if_bwfm_sdio.c
--- a/sys/dev/sdmmc/if_bwfm_sdio.c Thu May 21 16:50:25 2020 +0000
+++ b/sys/dev/sdmmc/if_bwfm_sdio.c Fri May 22 02:44:03 2020 +0000
@@ -69,7 +69,6 @@
struct bwfm_sdio_softc {
struct bwfm_softc sc_sc;
kmutex_t sc_lock;
- kmutex_t sc_intr_lock;
bool sc_bwfm_attached;
@@ -361,10 +360,8 @@
mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_NONE);
cv_init(&sc->sc_rxctl_cv, "bwfmctl");
- mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_NONE);
sdmmc_init_task(&sc->sc_task, bwfm_sdio_task, sc);
- sc->sc_task_queued = false;
sc->sc_bounce_size = 64 * 1024;
sc->sc_bounce_buf = kmem_alloc(sc->sc_bounce_size, KM_SLEEP);
@@ -620,10 +617,11 @@
if (sc->sc_bwfm_attached)
bwfm_detach(&sc->sc_sc, flags);
+ sdmmc_del_task(sc->sc_sf[1]->sc, &sc->sc_task, NULL);
+
kmem_free(sc->sc_sf, sc->sc_sf_size);
kmem_free(sc->sc_bounce_buf, sc->sc_bounce_size);
- mutex_destroy(&sc->sc_intr_lock);
cv_destroy(&sc->sc_rxctl_cv);
mutex_destroy(&sc->sc_lock);
@@ -1426,11 +1424,7 @@
DPRINTF(("%s: %s\n", DEVNAME(sc), name));
- mutex_enter(&sc->sc_intr_lock);
- if (!sdmmc_task_pending(&sc->sc_task))
- sdmmc_add_task(sc->sc_sf[1]->sc, &sc->sc_task);
- sc->sc_task_queued = true;
- mutex_exit(&sc->sc_intr_lock);
+ sdmmc_add_task(sc->sc_sf[1]->sc, &sc->sc_task);
return 1;
}
@@ -1444,33 +1438,13 @@
bwfm_sdio_task(void *v)
{
struct bwfm_sdio_softc *sc = (void *)v;
+
+ mutex_enter(&sc->sc_lock);
+ bwfm_sdio_task1(sc);
#ifdef BWFM_DEBUG
- unsigned count = 0;
-#endif
-
- mutex_enter(&sc->sc_intr_lock);
- while (sc->sc_task_queued) {
-#ifdef BWFM_DEBUG
- ++count;
+ bwfm_sdio_debug_console(sc);
#endif
- sc->sc_task_queued = false;
- mutex_exit(&sc->sc_intr_lock);
-
- mutex_enter(&sc->sc_lock);
- bwfm_sdio_task1(sc);
-#ifdef BWFM_DEBUG
- bwfm_sdio_debug_console(sc);
-#endif
- mutex_exit(&sc->sc_lock);
-
- mutex_enter(&sc->sc_intr_lock);
- }
- mutex_exit(&sc->sc_intr_lock);
-
-#ifdef BWFM_DEBUG
- if (count > 1)
- DPRINTF(("%s: finished %u tasks\n", DEVNAME(sc), count));
-#endif
+ mutex_exit(&sc->sc_lock);
}
static void
diff -r 21bda4d546c9 -r 02624e39f278 sys/dev/sdmmc/ld_sdmmc.c
--- a/sys/dev/sdmmc/ld_sdmmc.c Thu May 21 16:50:25 2020 +0000
+++ b/sys/dev/sdmmc/ld_sdmmc.c Fri May 22 02:44:03 2020 +0000
@@ -72,32 +72,35 @@
struct ld_sdmmc_task {
struct sdmmc_task task;
-
struct ld_sdmmc_softc *task_sc;
struct buf *task_bp;
int task_retries; /* number of xfer retry */
struct callout task_restart_ch;
- kmutex_t task_lock;
- kcondvar_t task_cv;
+ void *task_cookie;
- uintptr_t task_data;
+ TAILQ_ENTRY(ld_sdmmc_task) task_entry;
};
struct ld_sdmmc_softc {
struct ld_softc sc_ld;
int sc_hwunit;
-
+ char *sc_typename;
struct sdmmc_function *sc_sf;
- struct ld_sdmmc_task sc_task[LD_SDMMC_MAXTASKCNT];
- pcq_t *sc_freeq;
- char *sc_typename;
+
+ kmutex_t sc_lock;
+ kcondvar_t sc_cv;
+ TAILQ_HEAD(, ld_sdmmc_task) sc_freeq;
+ TAILQ_HEAD(, ld_sdmmc_task) sc_xferq;
+ bool sc_dying;
struct evcnt sc_ev_discard; /* discard counter */
struct evcnt sc_ev_discarderr; /* discard error counter */
struct evcnt sc_ev_discardbusy; /* discard busy counter */
struct evcnt sc_ev_cachesyncbusy; /* cache sync busy counter */
+
+ struct ld_sdmmc_task sc_task[LD_SDMMC_MAXTASKCNT];
};
static int ld_sdmmc_match(device_t, cfdata_t, void *);
@@ -157,15 +160,18 @@
evcnt_attach_dynamic(&sc->sc_ev_discardbusy, EVCNT_TYPE_MISC,
NULL, device_xname(self), "sdmmc discard busy");
+ mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SDMMC);
+ cv_init(&sc->sc_cv, "ldsdmmc");
+ TAILQ_INIT(&sc->sc_freeq);
+ TAILQ_INIT(&sc->sc_xferq);
+ sc->sc_dying = false;
+
const int ntask = __arraycount(sc->sc_task);
- sc->sc_freeq = pcq_create(ntask, KM_SLEEP);
for (i = 0; i < ntask; i++) {
task = &sc->sc_task[i];
task->task_sc = sc;
callout_init(&task->task_restart_ch, CALLOUT_MPSAFE);
- mutex_init(&task->task_lock, MUTEX_DEFAULT, IPL_NONE);
- cv_init(&task->task_cv, "ldsdmmctask");
- pcq_put(sc->sc_freeq, task);
+ TAILQ_INSERT_TAIL(&sc->sc_freeq, task, task_entry);
}
sc->sc_hwunit = 0; /* always 0? */
@@ -231,19 +237,68 @@
{
struct ld_sdmmc_softc *sc = device_private(dev);
struct ld_softc *ld = &sc->sc_ld;
+ struct ld_sdmmc_task *task;
+ struct buf *bp;
int rv, i;
- if ((rv = ldbegindetach(ld, flags)) != 0)
+ /* Block new xfers. */
+ sc->sc_dying = true;
+
+ /* Abort all pending tasks. */
+ mutex_enter(&sc->sc_lock);
+ while ((task = TAILQ_FIRST(&sc->sc_xferq)) != NULL) {
+ /*
+ * Try to abort the callout. If it already fired, it
+ * may have scheduled the task if so, so we have to
+ * continue anyway.
+ */
+ (void)callout_halt(&task->task_restart_ch, &sc->sc_lock);
+
+ /* Try to abort the task. If it already started, tough. */
+ if (!sdmmc_del_task(sc->sc_sf->sc, &task->task, &sc->sc_lock))
+ continue;
+
+ /*
+ * We aborted the task while it was still pending.
+ * Remove it from the queue and dissociate it from any
+ * I/O xfer.
+ */
+ TAILQ_REMOVE(&sc->sc_xferq, task, task_entry);
+ TAILQ_INSERT_TAIL(&sc->sc_freeq, task, task_entry);
+ bp = task->task_bp;
+ task->task_bp = NULL;
+
+ /*
+ * If the task was for an I/O xfer, fail the I/O xfer,
+ * with the softc lock dropped since this is a callback
+ * into arbitrary other subsystems.
+ */
+ if (bp) {
+ mutex_exit(&sc->sc_lock);
+ bp->b_error = ENXIO;
+ bp->b_resid = bp->b_bcount;
+ lddone(&sc->sc_ld, bp);
+ mutex_enter(&sc->sc_lock);
+ }
+ }
+ mutex_exit(&sc->sc_lock);
+
+ /* Do the ld detach dance. */
+ if ((rv = ldbegindetach(ld, flags)) != 0) {
+ /* Detach failed -- back out. */
+ sc->sc_dying = false;
return rv;
+ }
ldenddetach(ld);
- for (i = 0; i < __arraycount(sc->sc_task); i++) {
+ KASSERT(TAILQ_EMPTY(&sc->sc_xferq));
+
+ for (i = 0; i < __arraycount(sc->sc_task); i++)
callout_destroy(&sc->sc_task[i].task_restart_ch);
- mutex_destroy(&sc->sc_task[i].task_lock);
- cv_destroy(&sc->sc_task[i].task_cv);
- }
- pcq_destroy(sc->sc_freeq);
+ cv_destroy(&sc->sc_cv);
+ mutex_destroy(&sc->sc_lock);
+
evcnt_detach(&sc->sc_ev_discard);
evcnt_detach(&sc->sc_ev_discarderr);
evcnt_detach(&sc->sc_ev_discardbusy);
@@ -256,10 +311,16 @@
ld_sdmmc_start(struct ld_softc *ld, struct buf *bp)
{
struct ld_sdmmc_softc *sc = device_private(ld->sc_dv);
- struct ld_sdmmc_task *task = pcq_get(sc->sc_freeq);
+ struct ld_sdmmc_task *task;
+ int error;
- if (task == NULL)
- return EAGAIN;
+ mutex_enter(&sc->sc_lock);
+ if (sc->sc_dying || (task = TAILQ_FIRST(&sc->sc_freeq)) == NULL) {
+ error = EAGAIN;
+ goto out;
+ }
+ TAILQ_REMOVE(&sc->sc_freeq, task, task_entry);
+ TAILQ_INSERT_TAIL(&sc->sc_xferq, task, task_entry);
task->task_bp = bp;
task->task_retries = 0;
@@ -267,7 +328,8 @@
sdmmc_add_task(sc->sc_sf->sc, &task->task);
- return 0;
+out: mutex_exit(&sc->sc_lock);
+ return error;
}
static void
@@ -335,7 +397,12 @@
}
done:
- pcq_put(sc->sc_freeq, task);
+ /* Dissociate the task from the I/O xfer and release it. */
+ task->task_bp = NULL;
+ mutex_enter(&sc->sc_lock);
+ TAILQ_REMOVE(&sc->sc_xferq, task, task_entry);
+ TAILQ_INSERT_TAIL(&sc->sc_freeq, task, task_entry);
+ mutex_exit(&sc->sc_lock);
lddone(&sc->sc_ld, bp);
}
@@ -364,11 +431,16 @@
/* An error from discard is non-fatal */
error = sdmmc_mem_discard(sc->sc_sf, sblkno, sblkno + nblks - 1);
- if (error != 0)
+
+ mutex_enter(&sc->sc_lock);
+ if (error)
sc->sc_ev_discarderr.ev_count++;
else
sc->sc_ev_discard.ev_count++;
- pcq_put(sc->sc_freeq, task);
+ task->task_bp = NULL;
+ TAILQ_REMOVE(&sc->sc_xferq, task, task_entry);
+ TAILQ_INSERT_TAIL(&sc->sc_freeq, task, task_entry);
+ mutex_exit(&sc->sc_lock);
if (error)
bp->b_error = error;
@@ -380,61 +452,106 @@
ld_sdmmc_discard(struct ld_softc *ld, struct buf *bp)
{
struct ld_sdmmc_softc *sc = device_private(ld->sc_dv);
- struct ld_sdmmc_task *task = pcq_get(sc->sc_freeq);
Home |
Main Index |
Thread Index |
Old Index