Date: Fri, 23 Mar 2012 11:38:21 -0700 From: Jason Wolfe <nitroboost@gmail.com> To: Mike Tancsa <mike@sentex.net> Cc: freebsd-net@freebsd.org, Adrian Chadd <adrian@freebsd.org>, Hooman Fazaeli <hoomanfazaeli@gmail.com>, John Baldwin <jhb@freebsd.org> Subject: Re: Intel 82574L interface wedging - em7.3.2/8.2-STABLE Message-ID: <CAAAm0r0OSJbWRAaEoPyHL1JDyAV6Q3sP_Ug%2BXXuX-5i23%2Bzx8A@mail.gmail.com> In-Reply-To: <4F6CBC59.2060601@sentex.net> References: <CAAAm0r3Qj%2B2rf8cx54bcyAXGQezcE8J=xXYPq4W-jDy75r8qew@mail.gmail.com> <201203151417.04507.jhb@freebsd.org> <CAAAm0r0JxpHdq9CS=uwWysojXv_K-iT_NOb_fTeKAiAE_w8nbQ@mail.gmail.com> <201203201457.24776.jhb@freebsd.org> <4F6CBC59.2060601@sentex.net>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Fri, Mar 23, 2012 at 11:09 AM, Mike Tancsa <mike@sentex.net> wrote: > On 3/20/2012 2:57 PM, John Baldwin wrote: >>>> TX when link becomes active. I've also updated it to fix resume for em >>>> and igb to DTRT when buf_ring is used, and to not include old-style start >>>> routines at all when using multiq. It is at >>>> http://www.freebsd.org/~jhb/patches/e1000_txeof2.patch >>> Thank for the patch sirs, so far it does look like it did the trick. >>> I'll know for certain here in a few days if I'm still in the clear. >>> I'm guessing after it goes through some more testing it'll be too late >>> to slip it into 8.3? >> >> Yes, this is too late for 8.3, but thanks for testing! > > Hi, > Is there a RELENG_8 version of this patch ? I have a server that used to > shows this issue quite a bit, but has not since 7.3.2. I would be happy > to stress it on the box. The patch above does not apply cleanly due to > the netmap diffs, but I can manually merge if thats the only difference. > > em1: <Intel(R) PRO/1000 Network Connection 7.3.2> port 0x2000-0x201f mem > 0xb4100000-0xb411ffff,0xb4120000-0xb4123fff irq 16 at device 0.0 on pci11 > em1: Using MSIX interrupts with 3 vectors > em1: [ITHREAD] > em1: [ITHREAD] > em1: [ITHREAD] > em1: Ethernet address: 00:15:17:ed:68:a4 > > em1@pci0:11:0:0: class=0x020000 card=0x34ec8086 chip=0x10d38086 > rev=0x00 hdr=0x00 > vendor = 'Intel Corporation' > device = 'Intel 82574L Gigabit Ethernet Controller (82574L)' > class = network > subclass = ethernet > bar [10] = type Memory, range 32, base 0xb4100000, size 131072, > enabled > bar [18] = type I/O Port, range 32, base 0x2000, size 32, enabled > bar [1c] = type Memory, range 32, base 0xb4120000, size 16384, enabled > cap 01[c8] = powerspec 2 supports D0 D3 current D0 > cap 05[d0] = MSI supports 1 message, 64 bit > cap 10[e0] = PCI-Express 1 endpoint max data 128(256) link x1(x1) > cap 11[a0] = MSI-X supports 5 messages in map 0x1c enabled > ecap 0001[100] = AER 1 0 fatal 0 non-fatal 0 corrected > ecap 0003[140] = Serial 1 001517ffffed68a4 > > > -- > ------------------- > Mike Tancsa, tel +1 519 651 3400 > Sentex Communications, mike@sentex.net > Providing Internet services since 1994 www.sentex.net > Cambridge, Ontario Canada http://www.tancsa.com/ Mike, Attached is my patch with the small issues you mention cleaned up. It worked for me against RELENG_8 (8.3-PRERELEASE) as of 4 days ago. On the testing front, I've been stable for those 4 days across the pool of test machines. Prior I couldn't get past 48 hours without an interface 'wedge'. Thanks again! Jason [-- Attachment #2 --] diff -ruN src.stock/sys/dev/e1000/if_em.c src/sys/dev/e1000/if_em.c --- src.stock/sys/dev/e1000/if_em.c 2012-01-31 15:47:10.000000000 -0700 +++ src/sys/dev/e1000/if_em.c 2012-03-18 23:08:03.000000000 -0700 @@ -193,13 +193,14 @@ static int em_shutdown(device_t); static int em_suspend(device_t); static int em_resume(device_t); -static void em_start(struct ifnet *); -static void em_start_locked(struct ifnet *, struct tx_ring *); #ifdef EM_MULTIQUEUE static int em_mq_start(struct ifnet *, struct mbuf *); static int em_mq_start_locked(struct ifnet *, struct tx_ring *, struct mbuf *); static void em_qflush(struct ifnet *); +#else +static void em_start(struct ifnet *); +static void em_start_locked(struct ifnet *, struct tx_ring *); #endif static int em_ioctl(struct ifnet *, u_long, caddr_t); static void em_init(void *); @@ -234,7 +235,7 @@ static void em_disable_intr(struct adapter *); static void em_update_stats_counters(struct adapter *); static void em_add_hw_stats(struct adapter *adapter); -static bool em_txeof(struct tx_ring *); +static void em_txeof(struct tx_ring *); static bool em_rxeof(struct rx_ring *, int, int *); #ifndef __NO_STRICT_ALIGNMENT static int em_fixup_rx(struct rx_ring *); @@ -836,6 +837,7 @@ em_resume(device_t dev) { struct adapter *adapter = device_get_softc(dev); + struct tx_ring *txr = adapter->tx_rings; struct ifnet *ifp = adapter->ifp; EM_CORE_LOCK(adapter); @@ -843,8 +845,22 @@ e1000_resume_workarounds_pchlan(&adapter->hw); em_init_locked(adapter); em_init_manageability(adapter); + + if ((ifp->if_flags & IFF_UP) && + (ifp->if_drv_flags & IFF_DRV_RUNNING) && adapter->link_active) { + for (int i = 0; i < adapter->num_queues; i++, txr++) { + EM_TX_LOCK(txr); +#ifdef EM_MULTIQUEUE + if (!drbr_empty(ifp, txr->br)) + em_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); +#endif + EM_TX_UNLOCK(txr); + } + } EM_CORE_UNLOCK(adapter); - em_start(ifp); return bus_generic_resume(dev); } @@ -948,7 +964,7 @@ } if_qflush(ifp); } -#endif /* EM_MULTIQUEUE */ +#else /* !EM_MULTIQUEUE */ static void em_start_locked(struct ifnet *ifp, struct tx_ring *txr) @@ -1009,14 +1025,9 @@ em_start_locked(ifp, txr); EM_TX_UNLOCK(txr); } - /* - ** If we went inactive schedule - ** a task to clean up. - */ - if (ifp->if_drv_flags & IFF_DRV_OACTIVE) - taskqueue_enqueue(txr->tq, &txr->tx_task); return; } +#endif /* EM_MULTIQUEUE */ /********************************************************************* * Ioctl entry point @@ -1413,7 +1424,8 @@ if (!drbr_empty(ifp, txr->br)) em_mq_start_locked(ifp, txr, NULL); #else - em_start_locked(ifp, txr); + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); #endif EM_TX_UNLOCK(txr); @@ -1486,10 +1498,11 @@ if (!drbr_empty(ifp, txr->br)) em_mq_start_locked(ifp, txr, NULL); #else - em_start_locked(ifp, txr); + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); #endif EM_TX_UNLOCK(txr); - if (more || (ifp->if_drv_flags & IFF_DRV_OACTIVE)) { + if (more) { taskqueue_enqueue(adapter->tq, &adapter->que_task); return; } @@ -1510,17 +1523,21 @@ { struct tx_ring *txr = arg; struct adapter *adapter = txr->adapter; - bool more; + struct ifnet *ifp = adapter->ifp; ++txr->tx_irq; EM_TX_LOCK(txr); - more = em_txeof(txr); + em_txeof(txr); +#ifdef EM_MULTIQUEUE + if (!drbr_empty(ifp, txr->br)) + em_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); +#endif + /* Reenable this interrupt */ + E1000_WRITE_REG(&adapter->hw, E1000_IMS, txr->ims); EM_TX_UNLOCK(txr); - if (more) - taskqueue_enqueue(txr->tq, &txr->tx_task); - else - /* Reenable this interrupt */ - E1000_WRITE_REG(&adapter->hw, E1000_IMS, txr->ims); return; } @@ -1598,7 +1615,8 @@ if (!drbr_empty(ifp, txr->br)) em_mq_start_locked(ifp, txr, NULL); #else - em_start_locked(ifp, txr); + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); #endif E1000_WRITE_REG(&adapter->hw, E1000_IMS, txr->ims); EM_TX_UNLOCK(txr); @@ -1608,6 +1626,7 @@ em_handle_link(void *context, int pending) { struct adapter *adapter = context; + struct tx_ring *txr = adapter->tx_rings; struct ifnet *ifp = adapter->ifp; if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) @@ -1619,6 +1638,19 @@ callout_reset(&adapter->timer, hz, em_local_timer, adapter); E1000_WRITE_REG(&adapter->hw, E1000_IMS, EM_MSIX_LINK | E1000_IMS_LSC); + if (adapter->link_active) { + for (int i = 0; i < adapter->num_queues; i++, txr++) { + EM_TX_LOCK(txr); +#ifdef EM_MULTIQUEUE + if (!drbr_empty(ifp, txr->br)) + em_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + em_start_locked(ifp, txr); +#endif + EM_TX_UNLOCK(txr); + } + } EM_CORE_UNLOCK(adapter); } @@ -2891,20 +2923,21 @@ ifp->if_softc = adapter; ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; ifp->if_ioctl = em_ioctl; +#ifdef EM_MULTIQUEUE + /* Multiqueue stack interface */ + ifp->if_transmit = em_mq_start; + ifp->if_qflush = em_qflush; +#else ifp->if_start = em_start; IFQ_SET_MAXLEN(&ifp->if_snd, adapter->num_tx_desc - 1); ifp->if_snd.ifq_drv_maxlen = adapter->num_tx_desc - 1; IFQ_SET_READY(&ifp->if_snd); +#endif ether_ifattach(ifp, adapter->hw.mac.addr); ifp->if_capabilities = ifp->if_capenable = 0; -#ifdef EM_MULTIQUEUE - /* Multiqueue stack interface */ - ifp->if_transmit = em_mq_start; - ifp->if_qflush = em_qflush; -#endif ifp->if_capabilities |= IFCAP_HWCSUM | IFCAP_VLAN_HWCSUM; ifp->if_capabilities |= IFCAP_TSO4; @@ -3710,7 +3743,7 @@ * tx_buffer is put back on the free queue. * **********************************************************************/ -static bool +static void em_txeof(struct tx_ring *txr) { struct adapter *adapter = txr->adapter; @@ -3724,7 +3757,7 @@ /* No work, make sure watchdog is off */ if (txr->tx_avail == adapter->num_tx_desc) { txr->queue_status = EM_QUEUE_IDLE; - return (FALSE); + return; } processed = 0; @@ -3813,10 +3846,7 @@ /* Disable watchdog if all clean */ if (txr->tx_avail == adapter->num_tx_desc) { txr->queue_status = EM_QUEUE_IDLE; - return (FALSE); } - - return (TRUE); } diff -ruN src.stock/sys/dev/e1000/if_igb.c src/sys/dev/e1000/if_igb.c --- src.stock/sys/dev/e1000/if_igb.c 2012-01-31 15:47:10.000000000 -0700 +++ src/sys/dev/e1000/if_igb.c 2012-03-18 21:45:15.000000000 -0700 @@ -171,13 +171,14 @@ static int igb_shutdown(device_t); static int igb_suspend(device_t); static int igb_resume(device_t); -static void igb_start(struct ifnet *); -static void igb_start_locked(struct tx_ring *, struct ifnet *ifp); #if __FreeBSD_version >= 800000 static int igb_mq_start(struct ifnet *, struct mbuf *); static int igb_mq_start_locked(struct ifnet *, struct tx_ring *, struct mbuf *); static void igb_qflush(struct ifnet *); +#else +static void igb_start(struct ifnet *); +static void igb_start_locked(struct tx_ring *, struct ifnet *ifp); #endif static int igb_ioctl(struct ifnet *, u_long, caddr_t); static void igb_init(void *); @@ -261,6 +262,7 @@ static void igb_msix_link(void *); static void igb_handle_que(void *context, int pending); static void igb_handle_link(void *context, int pending); +static void igb_handle_link_locked(struct adapter *); static void igb_set_sysctl_value(struct adapter *, const char *, const char *, int *, int); @@ -798,6 +800,7 @@ igb_resume(device_t dev) { struct adapter *adapter = device_get_softc(dev); + struct tx_ring *txr = adapter->tx_rings; struct ifnet *ifp = adapter->ifp; IGB_CORE_LOCK(adapter); @@ -805,9 +808,21 @@ igb_init_manageability(adapter); if ((ifp->if_flags & IFF_UP) && - (ifp->if_drv_flags & IFF_DRV_RUNNING)) - igb_start(ifp); - + (ifp->if_drv_flags & IFF_DRV_RUNNING) && adapter->link_active) { + for (int i = 0; i < adapter->num_queues; i++, txr++) { + IGB_TX_LOCK(txr); +#if __FreeBSD_version >= 800000 + /* Process the stack queue only if not depleted */ + if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) && + !drbr_empty(ifp, txr->br)) + igb_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + igb_start_locked(txr, ifp); +#endif + IGB_TX_UNLOCK(txr); + } + } IGB_CORE_UNLOCK(adapter); return bus_generic_resume(dev); @@ -1321,19 +1336,19 @@ more = igb_rxeof(que, adapter->rx_process_limit, NULL); IGB_TX_LOCK(txr); - if (igb_txeof(txr)) - more = TRUE; + igb_txeof(txr); #if __FreeBSD_version >= 800000 /* Process the stack queue only if not depleted */ if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) && !drbr_empty(ifp, txr->br)) igb_mq_start_locked(ifp, txr, NULL); #else - igb_start_locked(txr, ifp); + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + igb_start_locked(txr, ifp); #endif IGB_TX_UNLOCK(txr); /* Do we need another? */ - if (more || (ifp->if_drv_flags & IFF_DRV_OACTIVE)) { + if (more) { taskqueue_enqueue(que->tq, &que->que_task); return; } @@ -1356,8 +1371,35 @@ { struct adapter *adapter = context; + IGB_CORE_LOCK(adapter); + igb_handle_link_locked(adapter); + IGB_CORE_UNLOCK(adapter); +} + +static void +igb_handle_link_locked(struct adapter *adapter) +{ + struct tx_ring *txr = adapter->tx_rings; + struct ifnet *ifp = adapter->ifp; + + IGB_CORE_LOCK_ASSERT(adapter); adapter->hw.mac.get_link_status = 1; igb_update_link_status(adapter); + if ((ifp->if_drv_flags & IFF_DRV_RUNNING) && adapter->link_active) { + for (int i = 0; i < adapter->num_queues; i++, txr++) { + IGB_TX_LOCK(txr); +#if __FreeBSD_version >= 800000 + /* Process the stack queue only if not depleted */ + if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) && + !drbr_empty(ifp, txr->br)) + igb_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + igb_start_locked(txr, ifp); +#endif + IGB_TX_UNLOCK(txr); + } + } } /********************************************************************* @@ -1437,7 +1479,7 @@ reg_icr = E1000_READ_REG(&adapter->hw, E1000_ICR); /* Link status change */ if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) - igb_handle_link(adapter, 0); + igb_handle_link_locked(adapter); if (reg_icr & E1000_ICR_RXO) adapter->rx_overruns++; @@ -1454,7 +1496,8 @@ if (!drbr_empty(ifp, txr->br)) igb_mq_start_locked(ifp, txr, NULL); #else - igb_start_locked(txr, ifp); + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + igb_start_locked(txr, ifp); #endif IGB_TX_UNLOCK(txr); return POLL_RETURN_COUNT(rx_done); @@ -1471,16 +1514,26 @@ { struct igb_queue *que = arg; struct adapter *adapter = que->adapter; + struct ifnet *ifp = adapter->ifp; struct tx_ring *txr = que->txr; struct rx_ring *rxr = que->rxr; u32 newitr = 0; - bool more_tx, more_rx; + bool more_rx; E1000_WRITE_REG(&adapter->hw, E1000_EIMC, que->eims); ++que->irqs; IGB_TX_LOCK(txr); - more_tx = igb_txeof(txr); + igb_txeof(txr); +#if __FreeBSD_version >= 800000 + /* Process the stack queue only if not depleted */ + if (((txr->queue_status & IGB_QUEUE_DEPLETED) == 0) && + !drbr_empty(ifp, txr->br)) + igb_mq_start_locked(ifp, txr, NULL); +#else + if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) + igb_start_locked(txr, ifp); +#endif IGB_TX_UNLOCK(txr); more_rx = igb_rxeof(que, adapter->rx_process_limit, NULL); @@ -1538,7 +1591,7 @@ no_calc: /* Schedule a clean task if needed*/ - if (more_tx || more_rx) + if (more_rx) taskqueue_enqueue(que->tq, &que->que_task); else /* Reenable this interrupt */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAAAm0r0OSJbWRAaEoPyHL1JDyAV6Q3sP_Ug%2BXXuX-5i23%2Bzx8A>