Author: np Date: Fri Apr 15 03:09:27 2011 New Revision: 220649 URL: http://svn.freebsd.org/changeset/base/220649
Log: Fix a couple of bad races that can occur when a cxgbe interface is taken down. The ingress queue lock was unused and has been removed as part of these changes. - An in-flight egress update from the SGE must be handled before the queue that requested it is destroyed. Wait for the update to arrive. - Interrupt handlers must stop processing rx events for a queue before the queue is destroyed. Events that have not yet been processed should be ignored once the queue disappears. MFC after: 1 week Modified: head/sys/dev/cxgbe/adapter.h head/sys/dev/cxgbe/common/t4_hw.c head/sys/dev/cxgbe/osdep.h head/sys/dev/cxgbe/t4_main.c head/sys/dev/cxgbe/t4_sge.c Modified: head/sys/dev/cxgbe/adapter.h ============================================================================== --- head/sys/dev/cxgbe/adapter.h Thu Apr 14 23:46:15 2011 (r220648) +++ head/sys/dev/cxgbe/adapter.h Fri Apr 15 03:09:27 2011 (r220649) @@ -227,6 +227,11 @@ enum { /* iq flags */ IQ_ALLOCATED = (1 << 1), /* firmware resources allocated */ IQ_STARTED = (1 << 2), /* started */ + + /* iq state */ + IQS_DISABLED = 0, + IQS_BUSY = 1, + IQS_IDLE = 2, }; /* @@ -244,7 +249,7 @@ struct sge_iq { iq_intr_handler_t *handler; __be64 *desc; /* KVA of descriptor ring */ - struct mtx iq_lock; + volatile uint32_t state; struct adapter *adapter; const __be64 *cdesc; /* current descriptor */ uint8_t gen; /* generation bit */ @@ -445,22 +450,12 @@ struct adapter { #define PORT_LOCK_ASSERT_OWNED(pi) mtx_assert(&(pi)->pi_lock, MA_OWNED) #define PORT_LOCK_ASSERT_NOTOWNED(pi) mtx_assert(&(pi)->pi_lock, MA_NOTOWNED) -#define IQ_LOCK(iq) mtx_lock(&(iq)->iq_lock) -#define IQ_UNLOCK(iq) mtx_unlock(&(iq)->iq_lock) -#define IQ_LOCK_ASSERT_OWNED(iq) mtx_assert(&(iq)->iq_lock, MA_OWNED) -#define IQ_LOCK_ASSERT_NOTOWNED(iq) mtx_assert(&(iq)->iq_lock, MA_NOTOWNED) - #define FL_LOCK(fl) mtx_lock(&(fl)->fl_lock) #define FL_TRYLOCK(fl) mtx_trylock(&(fl)->fl_lock) #define FL_UNLOCK(fl) mtx_unlock(&(fl)->fl_lock) #define FL_LOCK_ASSERT_OWNED(fl) mtx_assert(&(fl)->fl_lock, MA_OWNED) #define FL_LOCK_ASSERT_NOTOWNED(fl) mtx_assert(&(fl)->fl_lock, MA_NOTOWNED) -#define RXQ_LOCK(rxq) IQ_LOCK(&(rxq)->iq) -#define RXQ_UNLOCK(rxq) IQ_UNLOCK(&(rxq)->iq) -#define RXQ_LOCK_ASSERT_OWNED(rxq) IQ_LOCK_ASSERT_OWNED(&(rxq)->iq) -#define RXQ_LOCK_ASSERT_NOTOWNED(rxq) IQ_LOCK_ASSERT_NOTOWNED(&(rxq)->iq) - #define RXQ_FL_LOCK(rxq) FL_LOCK(&(rxq)->fl) #define RXQ_FL_UNLOCK(rxq) FL_UNLOCK(&(rxq)->fl) #define RXQ_FL_LOCK_ASSERT_OWNED(rxq) FL_LOCK_ASSERT_OWNED(&(rxq)->fl) @@ -586,6 +581,8 @@ void t4_intr_fwd(void *); void t4_intr_err(void *); void t4_intr_evt(void *); void t4_intr_data(void *); +void t4_evt_rx(void *); +void t4_eth_rx(void *); int t4_eth_tx(struct ifnet *, struct sge_txq *, struct mbuf *); void t4_update_fl_bufsize(struct ifnet *); Modified: head/sys/dev/cxgbe/common/t4_hw.c ============================================================================== --- head/sys/dev/cxgbe/common/t4_hw.c Thu Apr 14 23:46:15 2011 (r220648) +++ head/sys/dev/cxgbe/common/t4_hw.c Fri Apr 15 03:09:27 2011 (r220649) @@ -32,6 +32,8 @@ __FBSDID("$FreeBSD$"); #include "t4_regs_values.h" #include "t4fw_interface.h" +#undef msleep +#define msleep(x) DELAY((x) * 1000) /** * t4_wait_op_done_val - wait until an operation is completed Modified: head/sys/dev/cxgbe/osdep.h ============================================================================== --- head/sys/dev/cxgbe/osdep.h Thu Apr 14 23:46:15 2011 (r220648) +++ head/sys/dev/cxgbe/osdep.h Fri Apr 15 03:09:27 2011 (r220649) @@ -74,8 +74,6 @@ typedef boolean_t bool; #define false FALSE #define true TRUE -#undef msleep -#define msleep(x) DELAY((x) * 1000) #define mdelay(x) DELAY((x) * 1000) #define udelay(x) DELAY(x) Modified: head/sys/dev/cxgbe/t4_main.c ============================================================================== --- head/sys/dev/cxgbe/t4_main.c Thu Apr 14 23:46:15 2011 (r220648) +++ head/sys/dev/cxgbe/t4_main.c Fri Apr 15 03:09:27 2011 (r220649) @@ -1038,8 +1038,21 @@ static void cxgbe_qflush(struct ifnet *ifp) { struct port_info *pi = ifp->if_softc; + struct sge_txq *txq; + int i; + struct mbuf *m; - device_printf(pi->dev, "%s unimplemented.\n", __func__); + /* queues do not exist if !IFF_DRV_RUNNING. */ + if (ifp->if_drv_flags & IFF_DRV_RUNNING) { + for_each_txq(pi, i, txq) { + TXQ_LOCK(txq); + m_freem(txq->m); + while ((m = buf_ring_dequeue_sc(txq->eq.br)) != NULL) + m_freem(m); + TXQ_UNLOCK(txq); + } + } + if_qflush(ifp); } static int @@ -2673,8 +2686,11 @@ cxgbe_txq_start(void *arg, int count) struct sge_txq *txq = arg; TXQ_LOCK(txq); - txq->eq.flags &= ~EQ_CRFLUSHED; - txq_start(txq->ifp, txq); + if (txq->eq.flags & EQ_CRFLUSHED) { + txq->eq.flags &= ~EQ_CRFLUSHED; + txq_start(txq->ifp, txq); + } else + wakeup_one(txq); /* txq is going away, wakeup free_txq */ TXQ_UNLOCK(txq); } Modified: head/sys/dev/cxgbe/t4_sge.c ============================================================================== --- head/sys/dev/cxgbe/t4_sge.c Thu Apr 14 23:46:15 2011 (r220648) +++ head/sys/dev/cxgbe/t4_sge.c Fri Apr 15 03:09:27 2011 (r220649) @@ -279,7 +279,7 @@ t4_setup_adapter_iqs(struct adapter *sc) } } - handler = t4_intr_evt; + handler = t4_evt_rx; i = 0; /* forward fwq's interrupt to the first fiq */ } else { handler = NULL; @@ -345,7 +345,7 @@ t4_setup_eth_queues(struct port_info *pi device_get_nameunit(pi->dev), i); init_iq(&rxq->iq, sc, pi->tmr_idx, pi->pktc_idx, pi->qsize_rxq, RX_IQ_ESIZE, - sc->flags & INTR_FWD ? t4_intr_data: NULL, name); + sc->flags & INTR_FWD ? t4_eth_rx : NULL, name); snprintf(name, sizeof(name), "%s rxq%d-fl", device_get_nameunit(pi->dev), i); @@ -428,6 +428,9 @@ t4_intr_fwd(void *arg) int ndesc_pending = 0, ndesc_total = 0; int qid; + if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY)) + return; + while (is_new_response(iq, &ctrl)) { rmb(); @@ -460,6 +463,8 @@ t4_intr_fwd(void *arg) V_CIDXINC(ndesc_pending) | V_INGRESSQID((u32)iq->cntxt_id) | V_SEINTARM(iq->intr_params)); } + + atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE); } /* Deals with error interrupts */ @@ -479,6 +484,32 @@ void t4_intr_evt(void *arg) { struct sge_iq *iq = arg; + + if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY)) + return; + + t4_evt_rx(arg); + + atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE); +} + +void +t4_intr_data(void *arg) +{ + struct sge_iq *iq = arg; + + if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY)) + return; + + t4_eth_rx(arg); + + atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE); +} + +void +t4_evt_rx(void *arg) +{ + struct sge_iq *iq = arg; struct adapter *sc = iq->adapter; struct rsp_ctrl *ctrl; const struct rss_header *rss; @@ -537,7 +568,7 @@ t4_intr_evt(void *arg) } void -t4_intr_data(void *arg) +t4_eth_rx(void *arg) { struct sge_rxq *rxq = arg; struct sge_iq *iq = arg; @@ -1017,8 +1048,6 @@ alloc_iq_fl(struct port_info *pi, struct if (pi == NULL) pi = sc->port[0]; - mtx_init(&iq->iq_lock, iq->lockname, NULL, MTX_DEF); - len = iq->qsize * iq->esize; rc = alloc_ring(sc, len, &iq->desc_tag, &iq->desc_map, &iq->ba, (void **)&iq->desc); @@ -1148,6 +1177,7 @@ alloc_iq_fl(struct port_info *pi, struct } /* Enable IQ interrupts */ + atomic_store_rel_32(&iq->state, IQS_IDLE); t4_write_reg(sc, MYPF_REG(A_SGE_PF_GTS), V_SEINTARM(iq->intr_params) | V_INGRESSQID(iq->cntxt_id)); @@ -1179,6 +1209,10 @@ free_iq_fl(struct port_info *pi, struct return (rc); } iq->flags &= ~IQ_STARTED; + + /* Synchronize with the interrupt handler */ + while (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_DISABLED)) + pause("iqfree", hz / 1000); } if (iq->flags & IQ_ALLOCATED) { @@ -1196,9 +1230,6 @@ free_iq_fl(struct port_info *pi, struct free_ring(sc, iq->desc_tag, iq->desc_map, iq->ba, iq->desc); - if (mtx_initialized(&iq->iq_lock)) - mtx_destroy(&iq->iq_lock); - bzero(iq, sizeof(*iq)); if (fl) { @@ -1425,6 +1456,27 @@ free_txq(struct port_info *pi, struct sg struct sge_eq *eq = &txq->eq; if (eq->flags & (EQ_ALLOCATED | EQ_STARTED)) { + + /* + * Wait for the response to a credit flush if there's one + * pending. Clearing the flag tells handle_sge_egr_update or + * cxgbe_txq_start (depending on how far the response has made + * it) that they should ignore the response and wake up free_txq + * instead. + * + * The interface has been marked down by the time we get here + * (both IFF_UP and IFF_DRV_RUNNING cleared). qflush has + * emptied the tx buf_rings and we know nothing new is being + * queued for tx so we don't have to worry about a new credit + * flush request. + */ + TXQ_LOCK(txq); + if (eq->flags & EQ_CRFLUSHED) { + eq->flags &= ~EQ_CRFLUSHED; + msleep(txq, &eq->eq_lock, 0, "crflush", 0); + } + TXQ_UNLOCK(txq); + rc = -t4_eth_eq_free(sc, sc->mbox, sc->pf, 0, eq->cntxt_id); if (rc != 0) { device_printf(pi->dev, @@ -2444,13 +2496,14 @@ handle_sge_egr_update(struct adapter *sc struct port_info *pi; txq = (void *)s->eqmap[qid - s->eq_start]; - - KASSERT(txq->eq.flags & EQ_CRFLUSHED, - ("%s: tx queue %p not expecting an update.", __func__, txq)); - - pi = txq->ifp->if_softc; - taskqueue_enqueue(pi->tq, &txq->resume_tx); - txq->egr_update++; + TXQ_LOCK(txq); + if (txq->eq.flags & EQ_CRFLUSHED) { + pi = txq->ifp->if_softc; + taskqueue_enqueue(pi->tq, &txq->resume_tx); + txq->egr_update++; + } else + wakeup_one(txq); /* txq is going away, wakeup free_txq */ + TXQ_UNLOCK(txq); return (0); } _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"