Date: Thu, 8 Apr 2010 13:56:26 -0700 From: Pyun YongHyeon <pyunyh@gmail.com> To: Mike Tancsa <mike@sentex.net> Cc: freebsd-stable@freebsd.org, jfvogel@gmail.com Subject: Re: em driver regression Message-ID: <20100408205626.GN5734@michelle.cdnetworks.com> In-Reply-To: <201004081831.o38IVR3s043434@lava.sentex.ca> References: <201004081313.o38DD4JM041821@lava.sentex.ca> <7.1.0.9.0.20100408091756.10652be0@sentex.net> <201004081446.o38EkU7h042296@lava.sentex.ca> <20100408181741.GI5734@michelle.cdnetworks.com> <201004081831.o38IVR3s043434@lava.sentex.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
--WYTEVAkct0FjGQmd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Thu, Apr 08, 2010 at 02:31:18PM -0400, Mike Tancsa wrote:
> At 02:17 PM 4/8/2010, Pyun YongHyeon wrote:
>
> >Try this patch. It should fix the issue. It seems Jack forgot to
> >strip CRC bytes as old em(4) didn't strip it, probably to
> >workaround silicon bug of old em(4) controllers.
>
> Thanks! The attached patch does indeed fix the dhclient issue.
>
>
> >It seems there are also TX issues here. The system load is too high
> >and sometimes system is not responsive while TX is in progress.
> >Because I initiated TCP bulk transfers, TSO should reduce CPU load
> >a lot but it didn't so I guess it could also be related watchdog
> >timeouts you've seen. I'll see what can be done.
>
> Thanks for looking into that as well!!
>
> ---Mike
>
Mike,
Here is patch I'm working on. This patch fixes high system load and
system is very responsive as before. But it seems there is still
some TX issue here. Bulk UDP performance is very poor(< 700Mbps)
and I have no idea what caused this at this moment.
BTW, I have trouble to reproduce watchdog timeouts. I'm not sure
whether latest fix from Jack cured it. By chance does your
controller support multi TX/RX queues? You can check whether em(4)
uses multi-queues with "vmstat -i". If em(4) use multi-queue you
may have multiple irq output for em0.
--WYTEVAkct0FjGQmd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="em.load.patch"
Index: if_em.c
===================================================================
--- if_em.c (revision 206403)
+++ if_em.c (working copy)
@@ -812,6 +812,10 @@
return (err);
}
+ /* Call cleanup if number of TX descriptors low */
+ if (txr->tx_avail <= EM_TX_CLEANUP_THRESHOLD)
+ em_txeof(txr);
+
enq = 0;
if (m == NULL) {
next = drbr_dequeue(ifp, txr->br);
@@ -909,6 +913,10 @@
if (!adapter->link_active)
return;
+ /* Call cleanup if number of TX descriptors low */
+ if (txr->tx_avail <= EM_TX_CLEANUP_THRESHOLD)
+ em_txeof(txr);
+
while (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
IFQ_DRV_DEQUEUE(&ifp->if_snd, m_head);
@@ -1427,17 +1435,12 @@
struct ifnet *ifp = adapter->ifp;
struct tx_ring *txr = adapter->tx_rings;
struct rx_ring *rxr = adapter->rx_rings;
- u32 loop = EM_MAX_LOOP;
- bool more_rx, more_tx;
+ bool more_rx;
-
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
+ more_rx = em_rxeof(rxr, adapter->rx_process_limit);
EM_TX_LOCK(txr);
- do {
- more_rx = em_rxeof(rxr, adapter->rx_process_limit);
- more_tx = em_txeof(txr);
- } while (loop-- && (more_rx || more_tx));
-
+ em_txeof(txr);
#if __FreeBSD_version >= 800000
if (!drbr_empty(ifp, txr->br))
em_mq_start_locked(ifp, txr, NULL);
@@ -1445,10 +1448,9 @@
if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
em_start_locked(ifp, txr);
#endif
- if (more_rx || more_tx)
- taskqueue_enqueue(adapter->tq, &adapter->que_task);
-
EM_TX_UNLOCK(txr);
+ if (more_rx)
+ taskqueue_enqueue(adapter->tq, &adapter->que_task);
}
em_enable_intr(adapter);
@@ -1466,18 +1468,13 @@
{
struct tx_ring *txr = arg;
struct adapter *adapter = txr->adapter;
- bool more;
++txr->tx_irq;
EM_TX_LOCK(txr);
- more = em_txeof(txr);
+ em_txeof(txr);
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;
+ /* Reenable this interrupt */
+ E1000_WRITE_REG(&adapter->hw, E1000_IMS, txr->ims);
}
/*********************************************************************
@@ -1531,14 +1528,15 @@
{
struct rx_ring *rxr = context;
struct adapter *adapter = rxr->adapter;
- u32 loop = EM_MAX_LOOP;
bool more;
- do {
- more = em_rxeof(rxr, adapter->rx_process_limit);
- } while (loop-- && more);
- /* Reenable this interrupt */
- E1000_WRITE_REG(&adapter->hw, E1000_IMS, rxr->ims);
+ more = em_rxeof(rxr, adapter->rx_process_limit);
+ if (more)
+ taskqueue_enqueue(rxr->tq, &rxr->rx_task);
+ else {
+ /* Reenable this interrupt */
+ E1000_WRITE_REG(&adapter->hw, E1000_IMS, rxr->ims);
+ }
}
static void
@@ -1547,15 +1545,10 @@
struct tx_ring *txr = context;
struct adapter *adapter = txr->adapter;
struct ifnet *ifp = adapter->ifp;
- u32 loop = EM_MAX_LOOP;
- bool more;
if (!EM_TX_TRYLOCK(txr))
return;
- do {
- more = em_txeof(txr);
- } while (loop-- && more);
-
+ em_txeof(txr);
#if __FreeBSD_version >= 800000
if (!drbr_empty(ifp, txr->br))
em_mq_start_locked(ifp, txr, NULL);
@@ -1914,10 +1907,6 @@
E1000_WRITE_REG(&adapter->hw, E1000_TDT(txr->me), i);
txr->watchdog_time = ticks;
- /* Call cleanup if number of TX descriptors low */
- if (txr->tx_avail <= EM_TX_CLEANUP_THRESHOLD)
- em_txeof(txr);
-
return (0);
}
@@ -4078,7 +4067,7 @@
em_rxeof(struct rx_ring *rxr, int count)
{
struct adapter *adapter = rxr->adapter;
- struct ifnet *ifp = adapter->ifp;;
+ struct ifnet *ifp = adapter->ifp;
struct mbuf *mp, *sendmp;
u8 status;
u16 len;
@@ -4088,6 +4077,7 @@
EM_RX_LOCK(rxr);
+ status = 0;
for (i = rxr->next_to_check, processed = 0; count != 0;) {
if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
@@ -4195,7 +4185,11 @@
rxr->next_to_check = i;
EM_RX_UNLOCK(rxr);
+#ifdef DEVICE_POLLING
return (rxdone);
+#else
+ return ((status & E1000_RXD_STAT_DD) ? TRUE : FALSE);
+#endif
}
#ifndef __NO_STRICT_ALIGNMENT
--WYTEVAkct0FjGQmd--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100408205626.GN5734>