Date: Wed, 26 Sep 2012 09:15:56 -0700 From: Jack Vogel <jfvogel@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-net@freebsd.org, Vijay Singh <vijju.singh@gmail.com> Subject: Re: ixgbe rx & tx locks Message-ID: <CAFOYbck3MuBFndRr5cYaxcx2%2BUMGuMs8_Zy%2BODrUAQoekeSO4Q@mail.gmail.com> In-Reply-To: <201209260953.27806.jhb@freebsd.org> References: <CALCNsJSSQSWV7vNVR-Sn8CPDKbUBBLpSH0b-HYMJo3SXvkOY=w@mail.gmail.com> <201208170941.54482.jhb@freebsd.org> <CALCNsJQ740ceDzpd5n7QAALn-uJ-GdWxPTkQJuMJUMTUGJjOUg@mail.gmail.com> <201209260953.27806.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 26, 2012 at 6:53 AM, John Baldwin <jhb@freebsd.org> wrote:
> 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:
>
Thanks for the changes John, I approve the commit.
Jack
>
> 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?CAFOYbck3MuBFndRr5cYaxcx2%2BUMGuMs8_Zy%2BODrUAQoekeSO4Q>