Re: [PATCH v2] sh_eth: unmap DMA buffers when freeing rings

[David Miller]

From: Sergei Shtylyov 
Date: Wed, 19 Apr 2017 22:09:51 +0300

> On 04/17/2017 11:10 PM, David Miller wrote:
> 
>>> The DMA API debugging (when enabled) causes:
>>>
>>> WARNING: CPU: 0 PID: 1445 at lib/dma-debug.c:519
>>> add_dma_entry+0xe0/0x12c
>>> DMA-API: exceeded 7 overlapping mappings of cacheline 0x01b2974d
>>>
>>> to be  printed after repeated initialization of the Ether device, e.g.
>>> suspend/resume or 'ifconfig' up/down. This is because DMA buffers
>>> mapped
>>> using dma_map_single() in sh_eth_ring_format() and sh_eth_start_xmit()
>>> are
>>> never unmapped. Resolve this problem by unmapping the buffers when
>>> freeing
>>> the descriptor rings; in order to do it right, we'd have to add an
>>> extra
>>> parameter to sh_eth_txfree() (we rename this function to
>>> sh_eth_tx_free(),
>>> while at it).
>>>
>>> Based on the commit a47b70ea86bd ("ravb: unmap descriptors when
>>> freeing
>>> rings").
>>>
>>> Signed-off-by: Sergei Shtylyov 
>>
>> Applied, thanks.
> 
>Please don;t forget to send it to -stable.
>The bug seems to be there from the very beginning.

Ok, queued up.

Re: [PATCH v2] sh_eth: unmap DMA buffers when freeing rings

[Sergei Shtylyov]

On 04/17/2017 11:10 PM, David Miller wrote:


The DMA API debugging (when enabled) causes:

WARNING: CPU: 0 PID: 1445 at lib/dma-debug.c:519 add_dma_entry+0xe0/0x12c
DMA-API: exceeded 7 overlapping mappings of cacheline 0x01b2974d

to be  printed after repeated initialization of the Ether device, e.g.
suspend/resume or 'ifconfig' up/down. This is because DMA buffers mapped
using dma_map_single() in sh_eth_ring_format() and sh_eth_start_xmit() are
never unmapped. Resolve this problem by unmapping the buffers when freeing
the descriptor  rings;  in order  to do it right, we'd have to add an extra
parameter to sh_eth_txfree() (we rename this function to sh_eth_tx_free(),
while at it).

Based on the commit a47b70ea86bd ("ravb: unmap descriptors when freeing
rings").

Signed-off-by: Sergei Shtylyov 


Applied, thanks.


   Please don't forget to send this to -stable.
   The bug seems to be there from the very beginning.

MBR, Sergei

Re: [PATCH v2] sh_eth: unmap DMA buffers when freeing rings

[Sergei Shtylyov]

On 04/17/2017 11:10 PM, David Miller wrote:


The DMA API debugging (when enabled) causes:

WARNING: CPU: 0 PID: 1445 at lib/dma-debug.c:519 add_dma_entry+0xe0/0x12c
DMA-API: exceeded 7 overlapping mappings of cacheline 0x01b2974d

to be  printed after repeated initialization of the Ether device, e.g.
suspend/resume or 'ifconfig' up/down. This is because DMA buffers mapped
using dma_map_single() in sh_eth_ring_format() and sh_eth_start_xmit() are
never unmapped. Resolve this problem by unmapping the buffers when freeing
the descriptor  rings;  in order  to do it right, we'd have to add an extra
parameter to sh_eth_txfree() (we rename this function to sh_eth_tx_free(),
while at it).

Based on the commit a47b70ea86bd ("ravb: unmap descriptors when freeing
rings").

Signed-off-by: Sergei Shtylyov 


Applied, thanks.


   Please don;t forget to send it to -stable.
   The bug seems to be there from the very beginning.

MBR, Sergei

Re: [PATCH v2] sh_eth: unmap DMA buffers when freeing rings

[David Miller]

From: Sergei Shtylyov 
Date: Mon, 17 Apr 2017 15:55:22 +0300

> The DMA API debugging (when enabled) causes:
> 
> WARNING: CPU: 0 PID: 1445 at lib/dma-debug.c:519 add_dma_entry+0xe0/0x12c
> DMA-API: exceeded 7 overlapping mappings of cacheline 0x01b2974d
> 
> to be  printed after repeated initialization of the Ether device, e.g.
> suspend/resume or 'ifconfig' up/down. This is because DMA buffers mapped
> using dma_map_single() in sh_eth_ring_format() and sh_eth_start_xmit() are
> never unmapped. Resolve this problem by unmapping the buffers when freeing
> the descriptor  rings;  in order  to do it right, we'd have to add an extra
> parameter to sh_eth_txfree() (we rename this function to sh_eth_tx_free(),
> while at it).
> 
> Based on the commit a47b70ea86bd ("ravb: unmap descriptors when freeing
> rings").
> 
> Signed-off-by: Sergei Shtylyov 

Applied, thanks.

[PATCH v2] sh_eth: unmap DMA buffers when freeing rings

[Sergei Shtylyov]

The DMA API debugging (when enabled) causes:

WARNING: CPU: 0 PID: 1445 at lib/dma-debug.c:519 add_dma_entry+0xe0/0x12c
DMA-API: exceeded 7 overlapping mappings of cacheline 0x01b2974d

to be  printed after repeated initialization of the Ether device, e.g.
suspend/resume or 'ifconfig' up/down. This is because DMA buffers mapped
using dma_map_single() in sh_eth_ring_format() and sh_eth_start_xmit() are
never unmapped. Resolve this problem by unmapping the buffers when freeing
the descriptor  rings;  in order  to do it right, we'd have to add an extra
parameter to sh_eth_txfree() (we rename this function to sh_eth_tx_free(),
while at it).

Based on the commit a47b70ea86bd ("ravb: unmap descriptors when freeing
rings").

Signed-off-by: Sergei Shtylyov 

---
This patch is against DaveM's 'net.git' repo.

Changed in version 2:
- fixed the "packet sent" condition in sh_eth_tx_free().

 drivers/net/ethernet/renesas/sh_eth.c |  122 ++
 1 file changed, 67 insertions(+), 55 deletions(-)

Index: net/drivers/net/ethernet/renesas/sh_eth.c
===
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -1127,12 +1127,70 @@ static struct mdiobb_ops bb_ops = {
.get_mdio_data = sh_get_mdio,
 };
 
+/* free Tx skb function */
+static int sh_eth_tx_free(struct net_device *ndev, bool sent_only)
+{
+   struct sh_eth_private *mdp = netdev_priv(ndev);
+   struct sh_eth_txdesc *txdesc;
+   int free_num = 0;
+   int entry;
+   bool sent;
+
+   for (; mdp->cur_tx - mdp->dirty_tx > 0; mdp->dirty_tx++) {
+   entry = mdp->dirty_tx % mdp->num_tx_ring;
+   txdesc = >tx_ring[entry];
+   sent = !(txdesc->status & cpu_to_le32(TD_TACT));
+   if (sent_only && !sent)
+   break;
+   /* TACT bit must be checked before all the following reads */
+   dma_rmb();
+   netif_info(mdp, tx_done, ndev,
+  "tx entry %d status 0x%08x\n",
+  entry, le32_to_cpu(txdesc->status));
+   /* Free the original skb. */
+   if (mdp->tx_skbuff[entry]) {
+   dma_unmap_single(>dev, le32_to_cpu(txdesc->addr),
+le32_to_cpu(txdesc->len) >> 16,
+DMA_TO_DEVICE);
+   dev_kfree_skb_irq(mdp->tx_skbuff[entry]);
+   mdp->tx_skbuff[entry] = NULL;
+   free_num++;
+   }
+   txdesc->status = cpu_to_le32(TD_TFP);
+   if (entry >= mdp->num_tx_ring - 1)
+   txdesc->status |= cpu_to_le32(TD_TDLE);
+
+   if (sent) {
+   ndev->stats.tx_packets++;
+   ndev->stats.tx_bytes += le32_to_cpu(txdesc->len) >> 16;
+   }
+   }
+   return free_num;
+}
+
 /* free skb and descriptor buffer */
 static void sh_eth_ring_free(struct net_device *ndev)
 {
struct sh_eth_private *mdp = netdev_priv(ndev);
int ringsize, i;
 
+   if (mdp->rx_ring) {
+   for (i = 0; i < mdp->num_rx_ring; i++) {
+   if (mdp->rx_skbuff[i]) {
+   struct sh_eth_rxdesc *rxdesc = >rx_ring[i];
+
+   dma_unmap_single(>dev,
+le32_to_cpu(rxdesc->addr),
+ALIGN(mdp->rx_buf_sz, 32),
+DMA_FROM_DEVICE);
+   }
+   }
+   ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring;
+   dma_free_coherent(NULL, ringsize, mdp->rx_ring,
+ mdp->rx_desc_dma);
+   mdp->rx_ring = NULL;
+   }
+
/* Free Rx skb ringbuffer */
if (mdp->rx_skbuff) {
for (i = 0; i < mdp->num_rx_ring; i++)
@@ -1141,27 +1199,18 @@ static void sh_eth_ring_free(struct net_
kfree(mdp->rx_skbuff);
mdp->rx_skbuff = NULL;
 
-   /* Free Tx skb ringbuffer */
-   if (mdp->tx_skbuff) {
-   for (i = 0; i < mdp->num_tx_ring; i++)
-   dev_kfree_skb(mdp->tx_skbuff[i]);
-   }
-   kfree(mdp->tx_skbuff);
-   mdp->tx_skbuff = NULL;
-
-   if (mdp->rx_ring) {
-   ringsize = sizeof(struct sh_eth_rxdesc) * mdp->num_rx_ring;
-   dma_free_coherent(NULL, ringsize, mdp->rx_ring,
- mdp->rx_desc_dma);
-   mdp->rx_ring = NULL;
-   }
-
if (mdp->tx_ring) {
+   sh_eth_tx_free(ndev, false);
+
ringsize = sizeof(struct sh_eth_txdesc) * mdp->num_tx_ring;
dma_free_coherent(NULL, ringsize,