Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1533647 > unrolled thread

[PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers

Started byLino Sanfilippo <LinoSanfilippo@gmx.de>
First post2016-11-30 23:50 +0100
Last post2016-12-02 18:20 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-11-30 23:50 +0100
    [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler Lino Sanfilippo <LinoSanfilippo@gmx.de> - 2016-11-30 23:50 +0100
      Re: [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue  lock in tx completion handler David Miller <davem@davemloft.net> - 2016-12-02 18:20 +0100
    Re: [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma  sync for tx buffers David Miller <davem@davemloft.net> - 2016-12-02 18:20 +0100

#1533647 — [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers

FromLino Sanfilippo <LinoSanfilippo@gmx.de>
Date2016-11-30 23:50 +0100
Subject[PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers
Message-ID<sJlRo-3GR-7@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1533654 — [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler

FromLino Sanfilippo <LinoSanfilippo@gmx.de>
Date2016-11-30 23:50 +0100
Subject[PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler
Message-ID<sJlRo-3GR-25@gated-at.bofh.it>
In reply to#1533647
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

[toc] | [prev] | [next] | [standalone]


#1535090 — Re: [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler

FromDavid Miller <davem@davemloft.net>
Date2016-12-02 18:20 +0100
SubjectRe: [PATCH 2/2] net: ethernet: altera: TSE: do not use tx queue lock in tx completion handler
Message-ID<sJZF8-6IJ-21@gated-at.bofh.it>
In reply to#1533654
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.

[toc] | [prev] | [next] | [standalone]


#1535091 — Re: [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers

FromDavid Miller <davem@davemloft.net>
Date2016-12-02 18:20 +0100
SubjectRe: [PATCH 1/2] net: ethernet: altera: TSE: Remove unneeded dma sync for tx buffers
Message-ID<sJZF8-6IJ-25@gated-at.bofh.it>
In reply to#1533647
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.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web