DMA-API warning from sunhme - unchecked dma_map_single error

DMA-API warning from sunhme - unchecked dma_map_single error

[Meelis Roos]

I turned on DMA API debugging among other debug options to help catch an 
error. So far I have only found probably unrelated things. One of them 
is this splat:

WARNING: CPU: 6 PID: 1948 at lib/dma-debug.c:937 check_unmap+0x784/0x840()
hme 0000:00:01.1: DMA-API: device driver failed to check map error[device address=0x00000000c006e002] [size=342 bytes] [mapped as single]
Modules linked in:
CPU: 6 PID: 1948 Comm: dhclient Tainted: G        W    3.13.0-rc1 #16
Call Trace:
 [00000000004585cc] warn_slowpath_common+0x4c/0x80
 [00000000004586ac] warn_slowpath_fmt+0x2c/0x40
 [00000000006407e4] check_unmap+0x784/0x840
 [0000000000640944] debug_dma_unmap_page+0x44/0x60
 [000000000073f8c8] happy_meal_tx+0x128/0x260
 [000000000073fed8] happy_meal_interrupt+0x78/0xc0
 [00000000004a4b84] handle_irq_event_percpu+0x44/0x1a0
 [00000000004a4d18] handle_irq_event+0x38/0x80
 [00000000004a7dcc] handle_fasteoi_irq+0x10c/0x1c0
 [00000000004a4300] generic_handle_irq+0x40/0x60
 [000000000042bb38] handler_irq+0xb8/0xe0
 [0000000000426b2c] valid_addr_bitmap_patch+0x74/0x248
 [000000000049bf88] lock_acquire+0x68/0x80
 [0000000000494be4] down_read_trylock+0x44/0x60
 [000000000044bf54] do_sparc64_fault+0x1d4/0x700
 [0000000000407964] sparc64_realfault_common+0x10/0x20
---[ end trace 9a1420108ebfd592 ]---
Mapped at:
 [<0000000000740314>] happy_meal_start_xmit+0x1d4/0x4c0
 [<000000000077a8f0>] dev_hard_start_xmit+0x370/0x500
 [<0000000000795f8c>] sch_direct_xmit+0x4c/0x2a0
 [<000000000077ae18>] dev_queue_xmit+0x398/0x7c0
 [<00000000008015a8>] packet_sendmsg_spkt+0x388/0x4c0

It seems to be correct warning - dma_map_single is used unchecked in 
sunhme.c. I can try fixing it - the error handling will be the only 
problem. Is it considered worthwile?

-- 
Meelis Roos (mroos@linux.ee)

Re: DMA-API warning from sunhme - unchecked dma_map_single error

[David Miller]

From: Meelis Roos <mroos@linux.ee>
Date: Fri, 29 Nov 2013 10:40:40 +0200 (EET)

> It seems to be correct warning - dma_map_single is used unchecked in 
> sunhme.c. I can try fixing it - the error handling will be the only 
> problem. Is it considered worthwile?

Can you test this patch?

====================
sunhme: Add DMA mapping error checks.

Reported-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/ethernet/sun/sunhme.c | 62 +++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index 72c8525..9c01480 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -1262,6 +1262,7 @@ static void happy_meal_init_rings(struct happy_meal *hp)
 	HMD(("init rxring, "));
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		struct sk_buff *skb;
+		u32 mapping;
 
 		skb = happy_meal_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
 		if (!skb) {
@@ -1272,10 +1273,16 @@ static void happy_meal_init_rings(struct happy_meal *hp)
 
 		/* Because we reserve afterwards. */
 		skb_put(skb, (ETH_FRAME_LEN + RX_OFFSET + 4));
+		mapping = dma_map_single(hp->dma_dev, skb->data, RX_BUF_ALLOC_SIZE,
+					 DMA_FROM_DEVICE);
+		if (dma_mapping_error(hp->dma_dev, mapping)) {
+			dev_kfree_skb_any(skb);
+			hme_write_rxd(hp, &hb->happy_meal_rxd[i], 0, 0);
+			continue;
+		}
 		hme_write_rxd(hp, &hb->happy_meal_rxd[i],
 			      (RXFLAG_OWN | ((RX_BUF_ALLOC_SIZE - RX_OFFSET) << 16)),
-			      dma_map_single(hp->dma_dev, skb->data, RX_BUF_ALLOC_SIZE,
-					     DMA_FROM_DEVICE));
+			      mapping);
 		skb_reserve(skb, RX_OFFSET);
 	}
 
@@ -2020,6 +2027,7 @@ static void happy_meal_rx(struct happy_meal *hp, struct net_device *dev)
 		skb = hp->rx_skbs[elem];
 		if (len > RX_COPY_THRESHOLD) {
 			struct sk_buff *new_skb;
+			u32 mapping;
 
 			/* Now refill the entry, if we can. */
 			new_skb = happy_meal_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
@@ -2027,13 +2035,21 @@ static void happy_meal_rx(struct happy_meal *hp, struct net_device *dev)
 				drops++;
 				goto drop_it;
 			}
+			skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET + 4));
+			mapping = dma_map_single(hp->dma_dev, new_skb->data,
+						 RX_BUF_ALLOC_SIZE,
+						 DMA_FROM_DEVICE);
+			if (unlikely(dma_mapping_error(hp->dma_dev, mapping))) {
+				dev_kfree_skb_any(new_skb);
+				drops++;
+				goto drop_it;
+			}
+
 			dma_unmap_single(hp->dma_dev, dma_addr, RX_BUF_ALLOC_SIZE, DMA_FROM_DEVICE);
 			hp->rx_skbs[elem] = new_skb;
-			skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET + 4));
 			hme_write_rxd(hp, this,
 				      (RXFLAG_OWN|((RX_BUF_ALLOC_SIZE-RX_OFFSET)<<16)),
-				      dma_map_single(hp->dma_dev, new_skb->data, RX_BUF_ALLOC_SIZE,
-						     DMA_FROM_DEVICE));
+				      mapping);
 			skb_reserve(new_skb, RX_OFFSET);
 
 			/* Trim the original skb for the netif. */
@@ -2248,6 +2264,25 @@ static void happy_meal_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
+static void unmap_partial_tx_skb(struct happy_meal *hp, u32 first_mapping,
+				 u32 first_len, u32 first_entry, u32 entry)
+{
+	struct happy_meal_txd *txbase = &hp->happy_block->happy_meal_txd[0];
+
+	dma_unmap_single(hp->dma_dev, first_mapping, first_len, DMA_TO_DEVICE);
+
+	first_entry = NEXT_TX(first_entry);
+	while (first_entry != entry) {
+		struct happy_meal_txd *this = &txbase[first_entry];
+		u32 addr, len;
+
+		addr = hme_read_desc32(hp, &this->tx_addr);
+		len = hme_read_desc32(hp, &this->tx_flags);
+		len &= TXFLAG_SIZE;
+		dma_unmap_page(hp->dma_dev, addr, len, DMA_TO_DEVICE);
+	}
+}
+
 static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 					 struct net_device *dev)
 {
@@ -2284,6 +2319,8 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 
 		len = skb->len;
 		mapping = dma_map_single(hp->dma_dev, skb->data, len, DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(hp->dma_dev, mapping)))
+			goto out_dma_error;
 		tx_flags |= (TXFLAG_SOP | TXFLAG_EOP);
 		hme_write_txd(hp, &hp->happy_block->happy_meal_txd[entry],
 			      (tx_flags | (len & TXFLAG_SIZE)),
@@ -2299,6 +2336,8 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 		first_len = skb_headlen(skb);
 		first_mapping = dma_map_single(hp->dma_dev, skb->data, first_len,
 					       DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(hp->dma_dev, first_mapping)))
+			goto out_dma_error;
 		entry = NEXT_TX(entry);
 
 		for (frag = 0; frag < skb_shinfo(skb)->nr_frags; frag++) {
@@ -2308,6 +2347,11 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 			len = skb_frag_size(this_frag);
 			mapping = skb_frag_dma_map(hp->dma_dev, this_frag,
 						   0, len, DMA_TO_DEVICE);
+			if (unlikely(dma_mapping_error(hp->dma_dev, mapping))) {
+				unmap_partial_tx_skb(hp, first_mapping, first_len,
+						     first_entry, entry);
+				goto out_dma_error;
+			}
 			this_txflags = tx_flags;
 			if (frag == skb_shinfo(skb)->nr_frags - 1)
 				this_txflags |= TXFLAG_EOP;
@@ -2333,6 +2377,14 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 
 	tx_add_log(hp, TXLOG_ACTION_TXMIT, 0);
 	return NETDEV_TX_OK;
+
+out_dma_error:
+	hp->tx_skbs[hp->tx_new] = NULL;
+	spin_unlock_irq(&hp->happy_lock);
+
+	dev_kfree_skb_any(skb);
+	dev->stats.tx_dropped++;
+	return NETDEV_TX_OK;
 }
 
 static struct net_device_stats *happy_meal_get_stats(struct net_device *dev)
-- 
1.9.3

Re: DMA-API warning from sunhme - unchecked dma_map_single error

[Meelis Roos]

>> It seems to be correct warning - dma_map_single is used unchecked in
>> sunhme.c. I can try fixing it - the error handling will be the only
>> problem. Is it considered worthwile?
>
> Can you test this patch?

It works fine on E3500 where I had many debugging options on and caught 
it. No warnings, network still works.

-- 
Meelis Roos (mroos@linux.ee)

Re: DMA-API warning from sunhme - unchecked dma_map_single error

[David Miller]

From: Meelis Roos <mroos@linux.ee>
Date: Fri, 31 Oct 2014 23:01:09 +0200 (EET)

>>> It seems to be correct warning - dma_map_single is used unchecked in
>>> sunhme.c. I can try fixing it - the error handling will be the only
>>> problem. Is it considered worthwile?
>>
>> Can you test this patch?
> 
> It works fine on E3500 where I had many debugging options on and
> caught it. No warnings, network still works.

Thanks for testing, I'll push this to Linus and -stable soon.