DF-4606 / fix.diff
diff --git a/sys/dev/disk/nata/ata-queue.c b/sys/dev/disk/nata/ata-queue.c index 1111111..2222222 100644 --- a/sys/dev/disk/nata/ata-queue.c +++ b/sys/dev/disk/nata/ata-queue.c @@ -549,12 +549,24 @@ void ata_timeout(struct ata_request *request) { struct ata_channel *ch = device_get_softc(request->parent); /* * NOTE: callout acquired ch->state_mtx for us. */ /*request->flags |= ATA_R_DEBUG;*/ ATA_DEBUG_RQ(request, "timeout"); /* * if we have an ATA_ACTIVE request running, we flag the request * ATA_R_TIMEOUT so ata_finish will handle it correctly * also NULL out the running request so we wont loose * the race with an eventual interrupt arriving late */ if (ch->state == ATA_ACTIVE) { request->flags |= ATA_R_TIMEOUT; + /* + * DF-4606: NULL out ch->running while we still hold state_mtx -- + * the comment above claims this is done, but the code never did it. + * Leaving ch->running set opens a window between the LK_RELEASE + * below and the LK_EXCLUSIVE at the bottom of this function in + * which a late ata_interrupt() (ata-all.c:319-363), ata_reinit() + * (ata-all.c:206-208) or ata_fail_requests() (ata-queue.c:577-582) + * can take state_mtx and reach callout_cancel(&request->callout) + * while this callback is INPROG. _callout_cancel_or_stop(sync=1) + * then ssleep()s holding state_mtx waiting for this callback to + * return, while this callback blocks re-acquiring state_mtx: + * permanent deadlock (softclock thread + channel lock lost). + * The same window also lets a late interrupt complete the request + * a second time (double ata_finish / double ATA_LF_UNLOCK). + */ + ch->running = NULL; lockmgr(&ch->state_mtx, LK_RELEASE); ATA_LOCKING(ch->dev, ATA_LF_UNLOCK); ata_finish(request); lockmgr(&ch->state_mtx, LK_EXCLUSIVE); } } |