Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 26 Sep 2012 09:53:27 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Vijay Singh <vijju.singh@gmail.com>
Cc:        freebsd-net@freebsd.org, Jack Vogel <jfvogel@gmail.com>
Subject:   Re: ixgbe rx & tx locks
Message-ID:  <201209260953.27806.jhb@freebsd.org>
In-Reply-To: <CALCNsJQ740ceDzpd5n7QAALn-uJ-GdWxPTkQJuMJUMTUGJjOUg@mail.gmail.com>
References:  <CALCNsJSSQSWV7vNVR-Sn8CPDKbUBBLpSH0b-HYMJo3SXvkOY=w@mail.gmail.com> <201208170941.54482.jhb@freebsd.org> <CALCNsJQ740ceDzpd5n7QAALn-uJ-GdWxPTkQJuMJUMTUGJjOUg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday, September 25, 2012 4:19:01 pm Vijay Singh wrote:
> > Vijay, can you test this to see if it helps with your test case?
> >
> >> Jack
> 
> John, apologies for the delay. I have some data to share now.
> 
> With your patch, the transmit side lock contention is all gone.
> However I still see receive side contention. I have MSI/X enabled,
> with one hw queue per-port.

Well, that's progress at least.  Jack, can you ok this patch?  It is just the 
changes to adjust the deferred tx handling and doesn't include any watchdog 
changes.  The igb fix is just a comestic nit:

Index: dev/e1000/if_igb.h
===================================================================
--- dev/e1000/if_igb.h	(revision 240960)
+++ dev/e1000/if_igb.h	(working copy)
@@ -299,9 +299,9 @@
 	struct igb_tx_buffer	*tx_buffers;
 #if __FreeBSD_version >= 800000
 	struct buf_ring		*br;
+	struct task		txq_task;
 #endif
 	bus_dma_tag_t		txtag;
-	struct task		txq_task;
 
 	u32			bytes;
 	u32			packets;
Index: dev/ixgbe/ixgbe.c
===================================================================
--- dev/ixgbe/ixgbe.c	(revision 240960)
+++ dev/ixgbe/ixgbe.c	(working copy)
@@ -104,13 +104,15 @@
 static int      ixgbe_attach(device_t);
 static int      ixgbe_detach(device_t);
 static int      ixgbe_shutdown(device_t);
-static void     ixgbe_start(struct ifnet *);
-static void     ixgbe_start_locked(struct tx_ring *, struct ifnet *);
 #if __FreeBSD_version >= 800000
 static int	ixgbe_mq_start(struct ifnet *, struct mbuf *);
 static int	ixgbe_mq_start_locked(struct ifnet *,
                     struct tx_ring *, struct mbuf *);
 static void	ixgbe_qflush(struct ifnet *);
+static void	ixgbe_deferred_mq_start(void *, int);
+#else
+static void     ixgbe_start(struct ifnet *);
+static void     ixgbe_start_locked(struct tx_ring *, struct ifnet *);
 #endif
 static int      ixgbe_ioctl(struct ifnet *, u_long, caddr_t);
 static void	ixgbe_init(void *);
@@ -631,6 +633,7 @@
 {
 	struct adapter *adapter = device_get_softc(dev);
 	struct ix_queue *que = adapter->queues;
+	struct tx_ring *txr = adapter->tx_rings;
 	u32	ctrl_ext;
 
 	INIT_DEBUGOUT("ixgbe_detach: begin");
@@ -645,8 +648,11 @@
 	ixgbe_stop(adapter);
 	IXGBE_CORE_UNLOCK(adapter);
 
-	for (int i = 0; i < adapter->num_queues; i++, que++) {
+	for (int i = 0; i < adapter->num_queues; i++, que++, txr++) {
 		if (que->tq) {
+#if __FreeBSD_version >= 800000
+			taskqueue_drain(que->tq, &txr->txq_task);
+#endif
 			taskqueue_drain(que->tq, &que->que_task);
 			taskqueue_free(que->tq);
 		}
@@ -708,6 +714,7 @@
 }
 
 
+#if __FreeBSD_version < 800000
 /*********************************************************************
  *  Transmit entry point
  *
@@ -779,7 +786,7 @@
 	return;
 }
 
-#if __FreeBSD_version >= 800000
+#else
 /*
 ** Multiqueue Transmit driver
 **
@@ -807,7 +814,7 @@
 		IXGBE_TX_UNLOCK(txr);
 	} else {
 		err = drbr_enqueue(ifp, txr->br, m);
-		taskqueue_enqueue(que->tq, &que->que_task);
+		taskqueue_enqueue(que->tq, &txr->txq_task);
 	}
 
 	return (err);
@@ -873,6 +880,22 @@
 }
 
 /*
+ * Called from a taskqueue to drain queued transmit packets.
+ */
+static void
+ixgbe_deferred_mq_start(void *arg, int pending)
+{
+	struct tx_ring *txr = arg;
+	struct adapter *adapter = txr->adapter;
+	struct ifnet *ifp = adapter->ifp;
+
+	IXGBE_TX_LOCK(txr);
+	if (!drbr_empty(ifp, txr->br))
+		ixgbe_mq_start_locked(ifp, txr, NULL);
+	IXGBE_TX_UNLOCK(txr);
+}
+
+/*
 ** Flush all ring buffers
 */
 static void
@@ -2230,6 +2258,9 @@
 {
 	device_t dev = adapter->dev;
 	struct		ix_queue *que = adapter->queues;
+#if __FreeBSD_version >= 800000
+	struct tx_ring		*txr = adapter->tx_rings;
+#endif
 	int error, rid = 0;
 
 	/* MSI RID at 1 */
@@ -2249,6 +2280,9 @@
 	 * Try allocating a fast interrupt and the associated deferred
 	 * processing contexts.
 	 */
+#if __FreeBSD_version >= 800000
+	TASK_INIT(&txr->txq_task, 0, ixgbe_deferred_mq_start, txr);
+#endif
 	TASK_INIT(&que->que_task, 0, ixgbe_handle_que, que);
 	que->tq = taskqueue_create_fast("ixgbe_que", M_NOWAIT,
             taskqueue_thread_enqueue, &que->tq);
@@ -2295,9 +2329,10 @@
 {
 	device_t        dev = adapter->dev;
 	struct 		ix_queue *que = adapter->queues;
+	struct  	tx_ring *txr = adapter->tx_rings;
 	int 		error, rid, vector = 0;
 
-	for (int i = 0; i < adapter->num_queues; i++, vector++, que++) {
+	for (int i = 0; i < adapter->num_queues; i++, vector++, que++, txr++) {
 		rid = vector + 1;
 		que->res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid,
 		    RF_SHAREABLE | RF_ACTIVE);
@@ -2327,6 +2362,9 @@
 		if (adapter->num_queues > 1)
 			bus_bind_intr(dev, que->res, i);
 
+#if __FreeBSD_version >= 800000
+		TASK_INIT(&txr->txq_task, 0, ixgbe_deferred_mq_start, txr);
+#endif
 		TASK_INIT(&que->que_task, 0, ixgbe_handle_que, que);
 		que->tq = taskqueue_create_fast("ixgbe_que", M_NOWAIT,
 		    taskqueue_thread_enqueue, &que->tq);
@@ -2570,12 +2608,13 @@
 	ifp->if_softc = adapter;
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
 	ifp->if_ioctl = ixgbe_ioctl;
-	ifp->if_start = ixgbe_start;
 #if __FreeBSD_version >= 800000
 	ifp->if_transmit = ixgbe_mq_start;
 	ifp->if_qflush = ixgbe_qflush;
+#else
+	ifp->if_start = ixgbe_start;
+	IFQ_SET_MAXLEN(&ifp->if_snd, adapter->num_tx_desc - 2);
 #endif
-	ifp->if_snd.ifq_maxlen = adapter->num_tx_desc - 2;
 
 	ether_ifattach(ifp, adapter->hw.mac.addr);
 
Index: dev/ixgbe/ixgbe.h
===================================================================
--- dev/ixgbe/ixgbe.h	(revision 240960)
+++ dev/ixgbe/ixgbe.h	(working copy)
@@ -314,6 +314,7 @@
 	char			mtx_name[16];
 #if __FreeBSD_version >= 800000
 	struct buf_ring		*br;
+	struct task		txq_task;
 #endif
 #ifdef IXGBE_FDIR
 	u16			atr_sample;

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201209260953.27806.jhb>