linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers
@ 2016-11-30 22:48 Lino Sanfilippo
  2016-11-30 22:48 ` [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler Lino Sanfilippo
  2016-12-02 17:11 ` [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Lino Sanfilippo @ 2016-11-30 22:48 UTC (permalink / raw)
  To: vbridger; +Cc: nios2-dev, linux-kernel, netdev, Lino Sanfilippo

An explicit dma sync for device directly after mapping as well as an
explicit dma sync for cpu directly before unmapping is unnecessary and
costly on the hotpath. So remove these calls.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 10 ----------
 1 file changed, 10 deletions(-)

 Please note that this is only compile tested since I do not have the
 concerning hardware.

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index bda31f3..16c4163 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -400,12 +400,6 @@ static int tse_rx(struct altera_tse_private *priv, int limit)
 
 		skb_put(skb, pktlength);
 
-		/* make cache consistent with receive packet buffer */
-		dma_sync_single_for_cpu(priv->device,
-					priv->rx_ring[entry].dma_addr,
-					priv->rx_ring[entry].len,
-					DMA_FROM_DEVICE);
-
 		dma_unmap_single(priv->device, priv->rx_ring[entry].dma_addr,
 				 priv->rx_ring[entry].len, DMA_FROM_DEVICE);
 
@@ -592,10 +586,6 @@ static int tse_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	buffer->dma_addr = dma_addr;
 	buffer->len = nopaged_len;
 
-	/* Push data out of the cache hierarchy into main memory */
-	dma_sync_single_for_device(priv->device, buffer->dma_addr,
-				   buffer->len, DMA_TO_DEVICE);
-
 	priv->dmaops->tx_buffer(priv, buffer);
 
 	skb_tx_timestamp(skb);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler
  2016-11-30 22:48 [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers Lino Sanfilippo
@ 2016-11-30 22:48 ` Lino Sanfilippo
  2016-12-02 17:11   ` David Miller
  2016-12-02 17:11 ` [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Lino Sanfilippo @ 2016-11-30 22:48 UTC (permalink / raw)
  To: vbridger; +Cc: nios2-dev, linux-kernel, netdev, Lino Sanfilippo

The driver already uses its private lock for synchronization between xmit
and xmit completion handler making the additional use of the xmit_lock
unnecessary.
Furthermore the driver does not set NETIF_F_LLTX resulting in xmit to be
called with the xmit_lock held and then taking the private lock while xmit
completion handler does the reverse, first take the private lock, then the
xmit_lock.
Fix these issues by not taking the xmit_lock in the tx completion handler.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 2 --
 1 file changed, 2 deletions(-)

 Please note that this is only compile tested since I do not have the
 concerning hardware.

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 16c4163..cddc532 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -463,7 +463,6 @@ static int tse_tx_complete(struct altera_tse_private *priv)
 
 	if (unlikely(netif_queue_stopped(priv->dev) &&
 		     tse_tx_avail(priv) > TSE_TX_THRESH(priv))) {
-		netif_tx_lock(priv->dev);
 		if (netif_queue_stopped(priv->dev) &&
 		    tse_tx_avail(priv) > TSE_TX_THRESH(priv)) {
 			if (netif_msg_tx_done(priv))
@@ -471,7 +470,6 @@ static int tse_tx_complete(struct altera_tse_private *priv)
 					   __func__);
 			netif_wake_queue(priv->dev);
 		}
-		netif_tx_unlock(priv->dev);
 	}
 
 	spin_unlock(&priv->tx_lock);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers
  2016-11-30 22:48 [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers Lino Sanfilippo
  2016-11-30 22:48 ` [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler Lino Sanfilippo
@ 2016-12-02 17:11 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2016-12-02 17:11 UTC (permalink / raw)
  To: LinoSanfilippo; +Cc: vbridger, nios2-dev, linux-kernel, netdev

From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Wed, 30 Nov 2016 23:48:31 +0100

> An explicit dma sync for device directly after mapping as well as an
> explicit dma sync for cpu directly before unmapping is unnecessary and
> costly on the hotpath. So remove these calls.
> 
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

Applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler
  2016-11-30 22:48 ` [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler Lino Sanfilippo
@ 2016-12-02 17:11   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2016-12-02 17:11 UTC (permalink / raw)
  To: LinoSanfilippo; +Cc: vbridger, nios2-dev, linux-kernel, netdev

From: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Date: Wed, 30 Nov 2016 23:48:32 +0100

> The driver already uses its private lock for synchronization between xmit
> and xmit completion handler making the additional use of the xmit_lock
> unnecessary.
> Furthermore the driver does not set NETIF_F_LLTX resulting in xmit to be
> called with the xmit_lock held and then taking the private lock while xmit
> completion handler does the reverse, first take the private lock, then the
> xmit_lock.
> Fix these issues by not taking the xmit_lock in the tx completion handler.
> 
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

Yeah that could be a nasty deadlock, in fact.

Applied, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-12-02 17:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-30 22:48 [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers Lino Sanfilippo
2016-11-30 22:48 ` [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler Lino Sanfilippo
2016-12-02 17:11   ` David Miller
2016-12-02 17:11 ` [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).