[PATCH v2] bgmac: don't update slot on skb alloc/dma mapping error

Subsystems: broadcom iproc gbit ethernet driver, networking drivers, the rest

STALE4662d REVIEWED: 4 (4M)

1 review trailer.

2 messages, 2 authors, 2013-10-30 · open the first message on its own page

[PATCH v2] bgmac: don't update slot on skb alloc/dma mapping error

From: Nathan Hintz <hidden>
Date: 2013-10-30 02:32:05

Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
skb alloc and dma mapping are successful; and free the newly allocated
skb if a dma mapping error occurs.  This relieves the caller of the need
to deduce/execute the appropriate cleanup action required when an error
occurs.

Signed-off-by: Nathan Hintz <redacted>
Acked-by: Rafał Miłecki <zajec5@gmail.com>

---

v2: Revised commit message
---
 drivers/net/ethernet/broadcom/bgmac.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 7eca5a1..11e5d8d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -252,25 +252,33 @@ static int bgmac_dma_rx_skb_for_slot(struct bgmac *bgmac,
 				     struct bgmac_slot_info *slot)
 {
 	struct device *dma_dev = bgmac->core->dma_dev;
+	struct sk_buff *skb;
+	dma_addr_t dma_addr;
 	struct bgmac_rx_header *rx;
 
 	/* Alloc skb */
-	slot->skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
-	if (!slot->skb)
+	skb = netdev_alloc_skb(bgmac->net_dev, BGMAC_RX_BUF_SIZE);
+	if (!skb)
 		return -ENOMEM;
 
 	/* Poison - if everything goes fine, hardware will overwrite it */
-	rx = (struct bgmac_rx_header *)slot->skb->data;
+	rx = (struct bgmac_rx_header *)skb->data;
 	rx->len = cpu_to_le16(0xdead);
 	rx->flags = cpu_to_le16(0xbeef);
 
 	/* Map skb for the DMA */
-	slot->dma_addr = dma_map_single(dma_dev, slot->skb->data,
-					BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
-	if (dma_mapping_error(dma_dev, slot->dma_addr)) {
+	dma_addr = dma_map_single(dma_dev, skb->data,
+				  BGMAC_RX_BUF_SIZE, DMA_FROM_DEVICE);
+	if (dma_mapping_error(dma_dev, dma_addr)) {
 		bgmac_err(bgmac, "DMA mapping error\n");
+		dev_kfree_skb(skb);
 		return -ENOMEM;
 	}
+
+	/* Update the slot */
+	slot->skb = skb;
+	slot->dma_addr = dma_addr;
+
 	if (slot->dma_addr & 0xC0000000)
 		bgmac_warn(bgmac, "DMA address using 0xC0000000 bit(s), it may need translation trick\n");
 
-- 
1.8.3.1

Re: [PATCH v2] bgmac: don't update slot on skb alloc/dma mapping error

From: David Miller <davem@davemloft.net>
Date: 2013-10-30 02:59:53

From: Nathan Hintz <redacted>
Date: Tue, 29 Oct 2013 19:32:01 -0700
Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
skb alloc and dma mapping are successful; and free the newly allocated
skb if a dma mapping error occurs.  This relieves the caller of the need
to deduce/execute the appropriate cleanup action required when an error
occurs.

Signed-off-by: Nathan Hintz <redacted>
Acked-by: Rafał Miłecki <zajec5@gmail.com>
Applied, thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help