2016-11-30 22:48:47

by Lino Sanfilippo

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

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 <[email protected]>
---
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

2016-11-30 22:48:45

by Lino Sanfilippo

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

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 <[email protected]>
---
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

2016-12-02 17:12:56

by David Miller

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

From: Lino Sanfilippo <[email protected]>
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 <[email protected]>

Yeah that could be a nasty deadlock, in fact.

Applied, thanks.

2016-12-02 17:19:39

by David Miller

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

From: Lino Sanfilippo <[email protected]>
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 <[email protected]>

Applied.