--- a/sys/bus/firewire/firewire.c +++ b/sys/bus/firewire/firewire.c @@ -41,6 +41,9 @@ #include #include #include +#include +#include +#include #include /* used by smbus and newbus */ #include #include @@ -333,20 +336,64 @@ crit_enter(); for (i = 0; i < 0x40; i ++) { - while ((tl = STAILQ_FIRST(&fc->tlabels[i])) != NULL) { - xfer = tl->xfer; - if (timevalcmp(&xfer->tv, &tv, >)) - /* the rests are newer than this */ - break; - if (xfer->state == FWXF_START) - /* not sent yet */ + for (;;) { + struct fw_xfer *done_xfer; + + done_xfer = NULL; + spin_lock(&fc->tlabel_lock); + while ((tl = STAILQ_FIRST(&fc->tlabels[i])) != NULL) { + xfer = tl->xfer; + if (timevalcmp(&xfer->tv, &tv, >)) + /* the rests are newer than this */ + break; + if (xfer->state == FWXF_START) + /* not sent yet */ + break; + if (xfer->state == FWXF_RCVD) { + /* + * The response has already been + * delivered; the owner will + * complete and free the xfer. + * Drop only the tlabel, with + * its memory. + */ + STAILQ_REMOVE_HEAD(&fc->tlabels[i], + link); + kfree(tl, M_FW); + continue; + } + if (xfer->state == FWXF_SENT) { + u_char st = FWXF_SENT; + /* + * Atomically claim the completion + * so fw_rcv() on another CPU can- + * not complete (and the owner can- + * not free) the xfer under us. + */ + if (!atomic_fcmpset_char( + &xfer->state, &st, FWXF_RCVD)) + break; /* fw_rcv() won */ + } + device_printf(fc->bdev, + "split transaction timeout dst=0x%x tl=0x%x state=%d\n", + xfer->send.hdr.mode.hdr.dst, i, + xfer->state); + xfer->resp = ETIMEDOUT; + STAILQ_REMOVE_HEAD(&fc->tlabels[i], link); + /* the tlabel is off the list: fw_tl_free() + * will not find and free it anymore */ + kfree(tl, M_FW); + done_xfer = xfer; break; - device_printf(fc->bdev, - "split transaction timeout dst=0x%x tl=0x%x state=%d\n", - xfer->send.hdr.mode.hdr.dst, i, xfer->state); - xfer->resp = ETIMEDOUT; - STAILQ_REMOVE_HEAD(&fc->tlabels[i], link); - fw_xfer_done(xfer); + } + spin_unlock(&fc->tlabel_lock); + if (done_xfer != NULL) { + /* run the handler outside the lock: it + * may allocate or free the xfer */ + fw_xfer_done(done_xfer); + continue; + } + break; } } crit_exit(); @@ -756,6 +803,7 @@ /* Initialize Async handlers */ STAILQ_INIT(&fc->binds); + spin_init(&fc->tlabel_lock, "fwtlabel"); for( i = 0 ; i < 0x40 ; i++){ STAILQ_INIT(&fc->tlabels[i]); } @@ -892,7 +940,7 @@ { struct tlabel *tl; - crit_enter(); + spin_lock(&fc->tlabel_lock); for( tl = STAILQ_FIRST(&fc->tlabels[xfer->tl]); tl != NULL; tl = STAILQ_NEXT(tl, link)){ if(tl->xfer == xfer){ @@ -901,11 +949,16 @@ break; } } - crit_exit(); + spin_unlock(&fc->tlabel_lock); } /* - * To obtain XFER structure by transaction label. + * To obtain XFER structure by transaction label. On a FWXF_SENT xfer + * the completion is atomically claimed (FWXF_SENT -> FWXF_RCVD) under + * the tlabel lock, so firewire_xfer_timeout() on another CPU cannot + * complete (and the owner cannot free) the same xfer concurrently. + * Returns the xfer only when it was claimed, or when it is still + * FWXF_START (response raced transmission); NULL otherwise. */ static struct fw_xfer * fw_tl2xfer(struct firewire_comm *fc, int node, int tlabel) @@ -913,13 +966,24 @@ struct fw_xfer *xfer; struct tlabel *tl; - crit_enter(); + spin_lock(&fc->tlabel_lock); for( tl = STAILQ_FIRST(&fc->tlabels[tlabel]); tl != NULL; tl = STAILQ_NEXT(tl, link)){ if(tl->xfer->send.hdr.mode.hdr.dst == node){ xfer = tl->xfer; - crit_exit(); + if (xfer->state == FWXF_SENT) { + u_char st = FWXF_SENT; + if (!atomic_fcmpset_char(&xfer->state, + &st, FWXF_RCVD)) { + /* timeout is completing it */ + xfer = NULL; + } + } else if (xfer->state != FWXF_START) { + /* duplicate response / not completable */ + xfer = NULL; + } + spin_unlock(&fc->tlabel_lock); if (firewire_debug > 2) kprintf("fw_tl2xfer: found tl=%d\n", tlabel); return(xfer); @@ -927,7 +991,7 @@ } if (firewire_debug > 1) kprintf("fw_tl2xfer: not found tl=%d\n", tlabel); - crit_exit(); + spin_unlock(&fc->tlabel_lock); return(NULL); } @@ -1644,7 +1708,17 @@ struct tlabel *tl, *tmptl; static u_int32_t label = 0; - crit_enter(); + /* + * Allocate outside the lock (kmalloc(M_WAITOK) may sleep; this + * also removes the sleep from the critical section this function + * used to run in). + */ + tl = kmalloc(sizeof(struct tlabel), M_FW, M_WAITOK); + if (tl == NULL) + return(-1); + tl->xfer = xfer; + + spin_lock(&fc->tlabel_lock); for( i = 0 ; i < 0x40 ; i ++){ label = (label + 1) & 0x3f; for(tmptl = STAILQ_FIRST(&fc->tlabels[label]); @@ -1654,17 +1728,16 @@ break; } if(tmptl == NULL) { - tl = kmalloc(sizeof(struct tlabel), M_FW, M_WAITOK); - tl->xfer = xfer; STAILQ_INSERT_TAIL(&fc->tlabels[label], tl, link); - crit_exit(); + spin_unlock(&fc->tlabel_lock); if (firewire_debug > 1) kprintf("fw_get_tlabel: dst=%d tl=%d\n", xfer->send.hdr.mode.hdr.dst, label); return(label); } } - crit_exit(); + spin_unlock(&fc->tlabel_lock); + kfree(tl, M_FW); kprintf("fw_get_tlabel: no free tlabel\n"); return(-1); @@ -1776,27 +1849,22 @@ goto err; #endif } + /* + * fw_tl2xfer() claimed the completion (FWXF_SENT -> + * FWXF_RCVD) under the tlabel lock, or the xfer is still + * FWXF_START (response raced transmission). Copying is + * safe: the owner cannot free it before the completion. + */ + oldstate = rb->xfer->state; fw_rcv_copy(rb); if (rb->xfer->recv.hdr.mode.wres.rtcode != RESP_CMP) rb->xfer->resp = EIO; else rb->xfer->resp = 0; /* make sure the packet is drained in AT queue */ - oldstate = rb->xfer->state; - rb->xfer->state = FWXF_RCVD; - switch (oldstate) { - case FWXF_SENT: + if (oldstate == FWXF_RCVD) fw_xfer_done(rb->xfer); - break; - case FWXF_START: -#if 0 - if (firewire_debug) - kprintf("not sent yet tl=%x\n", rb->xfer->tl); -#endif - break; - default: - kprintf("unexpected state %d\n", rb->xfer->state); - } + /* FWXF_START: not sent yet, completion is left to TX */ return; case FWTCODE_WREQQ: case FWTCODE_WREQB: --- a/sys/bus/firewire/firewirereg.h +++ b/sys/bus/firewire/firewirereg.h @@ -128,6 +128,7 @@ struct fw_xferq *arq, *atq, *ars, *ats, *it[FW_MAX_DMACH],*ir[FW_MAX_DMACH]; STAILQ_HEAD(, tlabel) tlabels[0x40]; + struct spinlock tlabel_lock; /* guards tlabels[] lifetime */ STAILQ_HEAD(, fw_bind) binds; STAILQ_HEAD(, fw_device) devices; u_int sid_cnt;