svn commit: r191344 - head/sys/dev/xl

[Robert Watson]

On Tue, 21 Apr 2009, Pyun YongHyeon wrote:

>  Clear IFF_DRV_OACTIVE flag if one of queued packets was transmitted.
>  Previously it used to clear the flag only when the transmit queue
>  is empty which may slow down Tx performance.
>  While I'm here check whether driver is running and whether we can
>  queue more packets in if_start handler. This fixes occasional
>  watchdog timeouts.

Historically, IFF_DRV_OACTIVE was present to allow the driver to prevent calls 
to if_start when the driver was already in the transmit steady state (i.e., 
interrupt-driven transmit).  When the transmit interrupt saw it was out of 
queued packets and interrupt-driven transmission was ending, it would clear 
the flag so that future enqueues to the now-empty queue would generate an 
if_start to trigger interrupt-driven transmission again.  With that in mind -- 
are you sure this is the right change?  Shouldn't the descriptor ring be 
refilled when an interrupt comes in from the device after an appropriate 
interval?

Robert N M Watson
Computer Laboratory
University of Cambridge

>
>  Reported by:	xer < xernet <> hotmail dot it >
>  Tested by:	xer < xernet <> hotmail dot it >
>
> Modified:
>  head/sys/dev/xl/if_xl.c
>
> Modified: head/sys/dev/xl/if_xl.c
> ==============================================================================
> --- head/sys/dev/xl/if_xl.c	Mon Apr 20 23:25:38 2009	(r191343)
> +++ head/sys/dev/xl/if_xl.c	Tue Apr 21 00:34:31 2009	(r191344)
> @@ -2097,13 +2097,13 @@ xl_txeof(struct xl_softc *sc)
> 		m_freem(cur_tx->xl_mbuf);
> 		cur_tx->xl_mbuf = NULL;
> 		ifp->if_opackets++;
> +		ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
>
> 		cur_tx->xl_next = sc->xl_cdata.xl_tx_free;
> 		sc->xl_cdata.xl_tx_free = cur_tx;
> 	}
>
> 	if (sc->xl_cdata.xl_tx_head == NULL) {
> -		ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
> 		sc->xl_wdog_timer = 0;
> 		sc->xl_cdata.xl_tx_tail = NULL;
> 	} else {
> @@ -2540,6 +2540,9 @@ xl_start_locked(struct ifnet *ifp)
>
> 	XL_LOCK_ASSERT(sc);
>
> +	if ((ifp->if_drv_flags & (IFF_DRV_RUNNING | IFF_DRV_OACTIVE)) !=
> +	    IFF_DRV_RUNNING)
> +		return;
> 	/*
> 	 * Check for an available queue slot. If there are none,
> 	 * punt.
> @@ -2668,7 +2671,8 @@ xl_start_90xB_locked(struct ifnet *ifp)
>
> 	XL_LOCK_ASSERT(sc);
>
> -	if (ifp->if_drv_flags & IFF_DRV_OACTIVE)
> +	if ((ifp->if_drv_flags & (IFF_DRV_RUNNING | IFF_DRV_OACTIVE)) !=
> +	    IFF_DRV_RUNNING)
> 		return;
>
> 	idx = sc->xl_cdata.xl_tx_prod;
>