From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gw1.cosmosbay.com (gw1.cosmosbay.com [212.99.114.194]) by ozlabs.org (Postfix) with ESMTP id C0A28DDD0C for ; Fri, 27 Mar 2009 01:15:37 +1100 (EST) Message-ID: <49CB8DFD.2050504@cosmosbay.com> Date: Thu, 26 Mar 2009 15:15:25 +0100 From: Eric Dumazet MIME-Version: 1.0 To: Joakim Tjernlund Subject: Re: [PATCH 1/2] ucc_geth: Move freeing of TX packets to NAPI context. References: <1238072077-27044-1-git-send-email-Joakim.Tjernlund@transmode.se> In-Reply-To: <1238072077-27044-1-git-send-email-Joakim.Tjernlund@transmode.se> Content-Type: text/plain; charset=ISO-8859-1 Cc: linuxppc-dev@ozlabs.org, leoli@freescale.com, netdev@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Joakim Tjernlund a =E9crit : > Also set NAPI weight to 64 as this is a common value. > This will make the system alot more responsive while > ping flooding the ucc_geth ethernet interaface. >=20 > Signed-off-by: Joakim Tjernlund > --- > drivers/net/ucc_geth.c | 32 ++++++++++++-------------------- > drivers/net/ucc_geth.h | 1 - > 2 files changed, 12 insertions(+), 21 deletions(-) >=20 > diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c > index 097aed8..7fc91aa 100644 > --- a/drivers/net/ucc_geth.c > +++ b/drivers/net/ucc_geth.c > @@ -3214,7 +3214,7 @@ static int ucc_geth_tx(struct net_device *dev, u8= txQ) > dev->stats.tx_packets++; > =20 > /* Free the sk buffer associated with this TxBD */ > - dev_kfree_skb_irq(ugeth-> > + dev_kfree_skb(ugeth-> > tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]]); > ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]] =3D NULL; > ugeth->skb_dirtytx[txQ] =3D > @@ -3248,9 +3248,16 @@ static int ucc_geth_poll(struct napi_struct *nap= i, int budget) > for (i =3D 0; i < ug_info->numQueuesRx; i++) > howmany +=3D ucc_geth_rx(ugeth, i, budget - howmany); Not related to your patch, but seeing above code, I can understand you might have a problem in flood situation : Only first queue(s) are depleted, and last one cannot be because howmany >=3D budget. This driver might have to remember last queue it handled at previous call ucc_geth_poll(), so that all rxqueues have same probability to be sc= anned. j =3D ug->lastslot; for (i =3D 0; ug_info->numQueuesRx; i++) { if (++j >=3D ug_info->numQueuesRx) j =3D 0; howmany +=3D ucc_geth_rx(ugeth, j, budget - howmany); if (howmany >=3D budget) break; } ug->lastslot =3D j; > =20 > + /* Tx event processing */ > + spin_lock(&ugeth->lock); > + for (i =3D 0; i < ug_info->numQueuesTx; i++) { > + ucc_geth_tx(ugeth->dev, i); > + } > + spin_unlock(&ugeth->lock); > + > if (howmany < budget) { > netif_rx_complete(napi); > - setbits32(ugeth->uccf->p_uccm, UCCE_RX_EVENTS); > + setbits32(ugeth->uccf->p_uccm, UCCE_RX_EVENTS | UCCE_TX_EVENTS); > } > =20 > return howmany; > @@ -3264,8 +3271,6 @@ static irqreturn_t ucc_geth_irq_handler(int irq, = void *info) > struct ucc_geth_info *ug_info; > register u32 ucce; > register u32 uccm; > - register u32 tx_mask; > - u8 i; > =20 > ugeth_vdbg("%s: IN", __func__); > =20 > @@ -3279,27 +3284,14 @@ static irqreturn_t ucc_geth_irq_handler(int irq= , void *info) > out_be32(uccf->p_ucce, ucce); > =20 > /* check for receive events that require processing */ > - if (ucce & UCCE_RX_EVENTS) { > + if (ucce & (UCCE_RX_EVENTS | UCCE_TX_EVENTS)) { > if (netif_rx_schedule_prep(&ugeth->napi)) { > - uccm &=3D ~UCCE_RX_EVENTS; > + uccm &=3D ~(UCCE_RX_EVENTS | UCCE_TX_EVENTS); > out_be32(uccf->p_uccm, uccm); > __netif_rx_schedule(&ugeth->napi); > } > } > =20 > - /* Tx event processing */ > - if (ucce & UCCE_TX_EVENTS) { > - spin_lock(&ugeth->lock); > - tx_mask =3D UCC_GETH_UCCE_TXB0; > - for (i =3D 0; i < ug_info->numQueuesTx; i++) { > - if (ucce & tx_mask) > - ucc_geth_tx(dev, i); > - ucce &=3D ~tx_mask; > - tx_mask <<=3D 1; > - } > - spin_unlock(&ugeth->lock); > - } > - > /* Errors and other events */ > if (ucce & UCCE_OTHER) { > if (ucce & UCC_GETH_UCCE_BSY) > @@ -3733,7 +3725,7 @@ static int ucc_geth_probe(struct of_device* ofdev= , const struct of_device_id *ma > dev->netdev_ops =3D &ucc_geth_netdev_ops; > dev->watchdog_timeo =3D TX_TIMEOUT; > INIT_WORK(&ugeth->timeout_work, ucc_geth_timeout_work); > - netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, UCC_GETH_DEV_WEIGHT)= ; > + netif_napi_add(dev, &ugeth->napi, ucc_geth_poll, 64); > dev->mtu =3D 1500; > =20 > ugeth->msg_enable =3D netif_msg_init(debug.msg_enable, UGETH_MSG_DEFA= ULT); > diff --git a/drivers/net/ucc_geth.h b/drivers/net/ucc_geth.h > index 44218b8..50bad53 100644 > --- a/drivers/net/ucc_geth.h > +++ b/drivers/net/ucc_geth.h > @@ -843,7 +843,6 @@ struct ucc_geth_hardware_statistics { > /* Driver definitions */ > #define TX_BD_RING_LEN 0x10 > #define RX_BD_RING_LEN 0x10 > -#define UCC_GETH_DEV_WEIGHT TX_BD_RING_LEN > =20 > #define TX_RING_MOD_MASK(size) (size-1) > #define RX_RING_MOD_MASK(size) (size-1)