From: Stephen Irons
Sent: Tuesday, April 14, 2009 6:31 AM
>
> Do not re-enable interrupts if receive processing has used its entire
> budget.
>
> As described in net/core/dev.c, function net_rx_action, drivers must
> not
> modify
> the NAPI state if they consume their entire weight. Davinci EMAC
> driver was
> re-enabling interrupts in the rare case that it has used its budget,
> and
> there
> was now no more work to do.
>
> Tested on DM644x EVM.
> ---
> drivers/net/davinci_emac.c | 15 +++++++++------
> 1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> index 89ff48b..b3017ef 100755
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -2157,7 +2157,8 @@ static int emac_poll(struct napi_struct *napi,
> int
> budget)
> struct emac_priv *priv = container_of(napi, struct emac_priv,
> napi);
> struct net_device *ndev = priv->ndev;
> u32 status = 0;
> - u32 num_pkts = 0;
> + u32 num_txpkts = 0;
> + u32 num_rxpkts = 0;
> u32 txpending = 0;
> u32 rxpending = 0;
>
> @@ -2173,7 +2174,7 @@ static int emac_poll(struct napi_struct *napi,
> int
> budget)
> mask = EMAC_DM646X_MAC_IN_VECTOR_TX_INT_VEC;
>
> if (status & mask) {
> - num_pkts = emac_tx_bdproc(priv, EMAC_DEF_TX_CH,
> + num_txpkts = emac_tx_bdproc(priv, EMAC_DEF_TX_CH,
> EMAC_DEF_TX_MAX_SERVICE,
> &txpending);
> } /* TX processing */
> @@ -2184,7 +2185,7 @@ static int emac_poll(struct napi_struct *napi,
> int
> budget)
> mask = EMAC_DM646X_MAC_IN_VECTOR_RX_INT_VEC;
>
> if (status & mask) {
> - num_pkts = emac_rx_bdproc(priv, EMAC_DEF_RX_CH,
> + num_rxpkts = emac_rx_bdproc(priv, EMAC_DEF_RX_CH,
> budget, &rxpending);
> } /* RX processing */
>
> @@ -2194,8 +2195,10 @@ static int emac_poll(struct napi_struct *napi,
> int budget)
> __netif_rx_schedule(&priv->napi);
> }
> } else {
> - netif_rx_complete(napi);
> - emac_int_enable(priv);
> + if (num_rxpkts != budget) {
> + netif_rx_complete(napi);
> + emac_int_enable(priv);
> + }
Stephen,
Looking at the code, I think a better solution to the issue is to either
1) change the semantics of how the pending flag gets set in emac_rx_bdproc()
Or
2) get rid of the pending flag altogether and rely on the emac_rx_bdproc()
return value to make the decision on de-queuing ourselves.
Relying on both seems like a hack right now.
If you agree, you could submit an updated patch or we can take it up as part of
pushing the driver upstream which we intend to accelerate based on Kevin's
message.
Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
