| Message ID | 1413217302-15396-1-git-send-email-prashant@broadcom.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2014-10-13 at 09:21 -0700, Prashant Sreedharan wrote: > Ring TX doorbell only if xmit_more is not set or the queue is stopped. > > Suggested-by: Daniel Borkmann <dborkman@redhat.com> > Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> > --- > drivers/net/ethernet/broadcom/tg3.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) Have you noticed any performance change ? I did the patch for bnx2x but got no real difference... Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/13/2014 09:48 AM, Eric Dumazet wrote: > On Mon, 2014-10-13 at 09:21 -0700, Prashant Sreedharan wrote: >> Ring TX doorbell only if xmit_more is not set or the queue is stopped. >> >> Suggested-by: Daniel Borkmann <dborkman@redhat.com> >> Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> >> Signed-off-by: Michael Chan <mchan@broadcom.com> >> --- >> drivers/net/ethernet/broadcom/tg3.c | 10 ++++++---- >> 1 files changed, 6 insertions(+), 4 deletions(-) > > Have you noticed any performance change ? > > I did the patch for bnx2x but got no real difference... If ringing the doorbell is just a pio write, it may just get lost in the noise. If though the programming model (or some sort of defect workaround) of the NIC requires a pio read of some sort when ringing the doorbell... rick -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Prashant Sreedharan <prashant@broadcom.com> wrote: > Ring TX doorbell only if xmit_more is not set or the queue is stopped. > > Suggested-by: Daniel Borkmann <dborkman@redhat.com> > Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> > --- > drivers/net/ethernet/broadcom/tg3.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index ba49948..dbb41c1 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) > /* Sync BD data before updating mailbox */ > wmb(); > > - /* Packets are ready, update Tx producer idx local and on card. */ > - tw32_tx_mbox(tnapi->prodmbox, entry); > - > tnapi->tx_prod = entry; > if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) { > netif_tx_stop_queue(txq); > @@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) > netif_tx_wake_queue(txq); > } > > - mmiowb(); > + if (!skb->xmit_more || netif_xmit_stopped(txq)) { > + /* Packets are ready, update Tx producer idx on card. */ > + tw32_tx_mbox(tnapi->prodmbox, entry); I think you need to swap the test, i.e. netif_xmit_stopped() || xmit_more Otherwise, hw may not be aware of further packets wainting and queue might not be restarted? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Florian Westphal <fw@strlen.de> wrote: > Prashant Sreedharan <prashant@broadcom.com> wrote: > > Ring TX doorbell only if xmit_more is not set or the queue is stopped. > > > > Suggested-by: Daniel Borkmann <dborkman@redhat.com> > > Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> > > Signed-off-by: Michael Chan <mchan@broadcom.com> > > --- > > drivers/net/ethernet/broadcom/tg3.c | 10 ++++++---- > > 1 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > > index ba49948..dbb41c1 100644 > > --- a/drivers/net/ethernet/broadcom/tg3.c > > +++ b/drivers/net/ethernet/broadcom/tg3.c > > @@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) > > /* Sync BD data before updating mailbox */ > > wmb(); > > > > - /* Packets are ready, update Tx producer idx local and on card. */ > > - tw32_tx_mbox(tnapi->prodmbox, entry); > > - > > tnapi->tx_prod = entry; > > if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) { > > netif_tx_stop_queue(txq); > > @@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) > > netif_tx_wake_queue(txq); > > } > > > > - mmiowb(); > > + if (!skb->xmit_more || netif_xmit_stopped(txq)) { > > + /* Packets are ready, update Tx producer idx on card. */ > > + tw32_tx_mbox(tnapi->prodmbox, entry); > > I think you need to swap the test, i.e. Never mind, sorry for the noise. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Prashant Sreedharan <prashant@broadcom.com> Date: Mon, 13 Oct 2014 09:21:42 -0700 > Ring TX doorbell only if xmit_more is not set or the queue is stopped. > > Suggested-by: Daniel Borkmann <dborkman@redhat.com> > Signed-off-by: Prashant Sreedharan <prashant@broadcom.com> > Signed-off-by: Michael Chan <mchan@broadcom.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index ba49948..dbb41c1 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) /* Sync BD data before updating mailbox */ wmb(); - /* Packets are ready, update Tx producer idx local and on card. */ - tw32_tx_mbox(tnapi->prodmbox, entry); - tnapi->tx_prod = entry; if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) { netif_tx_stop_queue(txq); @@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) netif_tx_wake_queue(txq); } - mmiowb(); + if (!skb->xmit_more || netif_xmit_stopped(txq)) { + /* Packets are ready, update Tx producer idx on card. */ + tw32_tx_mbox(tnapi->prodmbox, entry); + mmiowb(); + } + return NETDEV_TX_OK; dma_error: