From: Jeff Garzik <jeff@garzik•org>
To: Auke Kok <auke-jan.h.kok@intel•com>
Cc: netdev@vger•kernel.org, akpm@linux-foundation•org, andi@firstfloor•org
Subject: Re: [PATCH 5/6] e1000e: error handling for pci_map_single calls.
Date: Tue, 14 Aug 2007 01:15:37 -0400 [thread overview]
Message-ID: <46C13A79.6050009@garzik.org> (raw)
In-Reply-To: <20070810200102.21509.97953.stgit@localhost.localdomain>
Auke Kok wrote:
> index d14cc4b..0e80406 100644
> --- a/drivers/net/e1000e/ethtool.c
> +++ b/drivers/net/e1000e/ethtool.c
> @@ -1042,6 +1044,10 @@ static int e1000_setup_desc_rings(struct e1000_adapter *adapter)
> tx_ring->buffer_info[i].dma =
> pci_map_single(pdev, skb->data, skb->len,
> PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(tx_ring->buffer_info[i].dma)) {
> + ret_val = 4;
> + goto err_nomem;
> + }
> tx_desc->buffer_addr = cpu_to_le64(
> tx_ring->buffer_info[i].dma);
> tx_desc->lower.data = cpu_to_le32(skb->len);
> @@ -1059,7 +1065,7 @@ static int e1000_setup_desc_rings(struct e1000_adapter *adapter)
> size = rx_ring->count * sizeof(struct e1000_buffer);
> rx_ring->buffer_info = kmalloc(size, GFP_KERNEL);
> if (!rx_ring->buffer_info) {
> - ret_val = 4;
> + ret_val = 5;
> goto err_nomem;
> }
> memset(rx_ring->buffer_info, 0, size);
> @@ -1068,7 +1074,7 @@ static int e1000_setup_desc_rings(struct e1000_adapter *adapter)
> rx_ring->desc = dma_alloc_coherent(&pdev->dev, rx_ring->size,
> &rx_ring->dma, GFP_KERNEL);
> if (!rx_ring->desc) {
> - ret_val = 5;
> + ret_val = 6;
> goto err_nomem;
> }
> memset(rx_ring->desc, 0, rx_ring->size);
> @@ -1093,7 +1099,7 @@ static int e1000_setup_desc_rings(struct e1000_adapter *adapter)
>
> skb = alloc_skb(2048 + NET_IP_ALIGN, GFP_KERNEL);
> if (!skb) {
> - ret_val = 6;
> + ret_val = 7;
> goto err_nomem;
> }
> skb_reserve(skb, NET_IP_ALIGN);
> @@ -1101,6 +1107,10 @@ static int e1000_setup_desc_rings(struct e1000_adapter *adapter)
> rx_ring->buffer_info[i].dma =
> pci_map_single(pdev, skb->data, 2048,
> PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(rx_ring->buffer_info[i].dma)) {
> + ret_val = 8;
> + goto err_nomem;
> + }
> rx_desc->buffer_addr =
> cpu_to_le64(rx_ring->buffer_info[i].dma);
> memset(skb->data, 0x00, skb->len);
this function is a bit fragile if two additional error checks cause a
'ret_val' ripple effect throughout the function.
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 51c9024..8ebe238 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -195,6 +195,11 @@ map_skb:
> buffer_info->dma = pci_map_single(pdev, skb->data,
> adapter->rx_buffer_len,
> PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(buffer_info->dma)) {
> + dev_err(&pdev->dev, "RX DMA map failed\n");
> + adapter->rx_dma_failed++;
> + break;
> + }
>
> rx_desc = E1000_RX_DESC(*rx_ring, i);
> rx_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
> @@ -255,6 +260,13 @@ static void e1000_alloc_rx_buffers_ps(struct e1000_adapter *adapter,
> ps_page->page,
> 0, PAGE_SIZE,
> PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(
> + ps_page->dma)) {
> + dev_err(&adapter->pdev->dev,
> + "RX DMA page map failed\n");
> + adapter->rx_dma_failed++;
> + goto no_buffers;
> + }
> }
> /*
> * Refresh the desc even if buffer_addrs
> @@ -286,6 +298,14 @@ static void e1000_alloc_rx_buffers_ps(struct e1000_adapter *adapter,
> buffer_info->dma = pci_map_single(pdev, skb->data,
> adapter->rx_ps_bsize0,
> PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(buffer_info->dma)) {
> + dev_err(&pdev->dev, "RX DMA map failed\n");
> + adapter->rx_dma_failed++;
> + /* cleanup skb */
> + dev_kfree_skb_any(skb);
> + buffer_info->skb = NULL;
> + break;
> + }
>
> rx_desc->read.buffer_addr[0] = cpu_to_le64(buffer_info->dma);
>
> @@ -374,6 +394,11 @@ check_page:
> buffer_info->page, 0,
> PAGE_SIZE,
> PCI_DMA_FROMDEVICE);
> + if (pci_dma_mapping_error(buffer_info->dma)) {
> + dev_err(&adapter->pdev->dev, "RX DMA page map failed\n");
> + adapter->rx_dma_failed++;
> + break;
> + }
>
> rx_desc = E1000_RX_DESC(*rx_ring, i);
> rx_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
> @@ -3214,6 +3239,11 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
> skb->data + offset,
> size,
> PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(buffer_info->dma)) {
> + dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
> + adapter->tx_dma_failed++;
> + return -1;
> + }
> buffer_info->next_to_watch = i;
>
> len -= size;
> @@ -3247,6 +3277,13 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
> offset,
> size,
> PCI_DMA_TODEVICE);
> + if (pci_dma_mapping_error(buffer_info->dma)) {
> + dev_err(&adapter->pdev->dev,
> + "TX DMA page map failed\n");
> + adapter->tx_dma_failed++;
> + return -1;
> + }
> +
> buffer_info->next_to_watch = i;
>
> len -= size;
> @@ -3512,9 +3549,15 @@ static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
> if (skb->protocol == htons(ETH_P_IP))
> tx_flags |= E1000_TX_FLAGS_IPV4;
>
> - e1000_tx_queue(adapter, tx_flags,
> - e1000_tx_map(adapter, skb, first,
> - max_per_txd, nr_frags, mss));
> + count = e1000_tx_map(adapter, skb, first, max_per_txd, nr_frags, mss);
> + if (count < 0) {
> + /* handle pci_map_single() error in e1000_tx_map */
> + dev_kfree_skb_any(skb);
> + spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
> + return NETDEV_TX_BUSY;
> + }
> +
> + e1000_tx_queue(adapter, tx_flags, count);
are you really supposed to free the skb, if you are returning
NETDEV_TX_BUSY? My memory is rusty but that seems strange
next prev parent reply other threads:[~2007-08-14 5:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-08-10 20:00 [PATCH 1/6] e1000e: Fix header includes [v2] Auke Kok
2007-08-10 20:00 ` [PATCH 2/6] e1000e: remove namespace collisions with e1000 Auke Kok
2007-08-14 5:13 ` Jeff Garzik
2007-08-14 16:41 ` Rick Jones
2007-08-14 21:22 ` Kok, Auke
2007-08-10 20:00 ` [PATCH 3/6] e1000e: Use dma_alloc_coherent where possible Auke Kok
2007-08-10 20:00 ` [PATCH 4/6] e1000e: Use time_after to account for jiffies wrapping Auke Kok
2007-08-10 20:01 ` [PATCH 5/6] e1000e: error handling for pci_map_single calls Auke Kok
2007-08-14 5:15 ` Jeff Garzik [this message]
2007-08-10 20:01 ` [PATCH 6/6] e1000e: Remove two compile warnings Auke Kok
2007-08-14 5:11 ` [PATCH 1/6] e1000e: Fix header includes [v2] Jeff Garzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=46C13A79.6050009@garzik.org \
--to=jeff@garzik$(echo .)org \
--cc=akpm@linux-foundation$(echo .)org \
--cc=andi@firstfloor$(echo .)org \
--cc=auke-jan.h.kok@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox