| Message ID | 12530821133392-git-send-email-jie.yang@atheros.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: <jie.yang@atheros.com> Date: Wed, 16 Sep 2009 14:21:53 +0800 > > [ 25.059969] WARNING: at lib/dma-debug.c:816 check_unmap+0x383/0x55c() > [ 25.059976] Hardware name: 1000HE > [ 25.059984] ATL1E 0000:03:00.0: DMA-API: device driver frees DMA > memory with wrong function [device address=0x0000000036b92802] > [size=90 bytes] [mapped as single] [unmapped as page] > > use the wrong API when free dma. So when map dma use a flag to demostrate > whether it is 'pci_map_single' or 'pci_map_page'. When free the dma, check > the flags to select the right APIs('pci_unmap_single' or 'pci_unmap_page'). > > Signed-off-by: Jie Yang <jie.yang@atheros.com> An 'unsigned long' is an enormous type to use just to store 3 bits of information. Use a u16 or similar to compact the size of struct atl1e_tx_buffer Also, it looks terribly ugle to define the flag macros, some with three leading zeros in the hexadecimal values and one without. Please make them consistent, thanks. Generally, I see that this change was put together very hastily and without very much care. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2009-09-16 at 14:21 +0800, jie.yang@atheros.com wrote: > - pci_unmap_page(pdev, tx_buffer->dma, > + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) > + pci_unmap_single(pdev, tx_buffer->dma, > + tx_buffer->length, PCI_DMA_TODEVICE); > + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) > + pci_unmap_page(pdev, tx_buffer->dma, Could you run this through checkpatch, and fix any errors if find? It looks like you uses spaces for tabs in the code above, maybe in the other block too.. Daniel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, September 16, 2009 5:11 PM Daniel Walker <dwalker@fifo99.com> wrote: > On Wed, 2009-09-16 at 14:21 +0800, jie.yang@atheros.com wrote: > > - pci_unmap_page(pdev, tx_buffer->dma, > > + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) > > + pci_unmap_single(pdev, tx_buffer->dma, > > + tx_buffer->length, PCI_DMA_TODEVICE); > > + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) > > + pci_unmap_page(pdev, tx_buffer->dma, > > Could you run this through checkpatch, and fix any errors if > find? It looks like you uses spaces for tabs in the code > above, maybe in the other block too.. > > Daniel > > ou, my mistake, just resend. Best wishes jie -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Daniel Walker <dwalker@fifo99.com> Date: Wed, 16 Sep 2009 02:10:51 -0700 > On Wed, 2009-09-16 at 14:21 +0800, jie.yang@atheros.com wrote: >> - pci_unmap_page(pdev, tx_buffer->dma, >> + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) >> + pci_unmap_single(pdev, tx_buffer->dma, >> + tx_buffer->length, PCI_DMA_TODEVICE); >> + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) >> + pci_unmap_page(pdev, tx_buffer->dma, > > Could you run this through checkpatch, and fix any errors if find? It > looks like you uses spaces for tabs in the code above, maybe in the > other block too.. > It's because of his email client, it corrupted the whole patch like that. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[David Miller - Wed, Sep 16, 2009 at 02:47:59AM -0700] | From: Daniel Walker <dwalker@fifo99.com> | Date: Wed, 16 Sep 2009 02:10:51 -0700 | | > On Wed, 2009-09-16 at 14:21 +0800, jie.yang@atheros.com wrote: | >> - pci_unmap_page(pdev, tx_buffer->dma, | >> + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) | >> + pci_unmap_single(pdev, tx_buffer->dma, | >> + tx_buffer->length, PCI_DMA_TODEVICE); | >> + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) | >> + pci_unmap_page(pdev, tx_buffer->dma, | > | > Could you run this through checkpatch, and fix any errors if find? It | > looks like you uses spaces for tabs in the code above, maybe in the | > other block too.. | > | | It's because of his email client, it corrupted the whole patch | like that. well, client seems to be | X-Mailer: git-send-email 1.5.2.2 more probably the text editor is the reason. -- Cyrill -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, September 16, 2009 5:52 PM Cyrill Gorcunov <gorcunov@gmail.com> wrote: > [David Miller - Wed, Sep 16, 2009 at 02:47:59AM -0700] > | From: Daniel Walker <dwalker@fifo99.com> > | Date: Wed, 16 Sep 2009 02:10:51 -0700 > | > | > On Wed, 2009-09-16 at 14:21 +0800, jie.yang@atheros.com wrote: > | >> - pci_unmap_page(pdev, tx_buffer->dma, > | >> + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) > | >> + pci_unmap_single(pdev, tx_buffer->dma, > | >> + tx_buffer->length, PCI_DMA_TODEVICE); > | >> + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) > | >> + pci_unmap_page(pdev, tx_buffer->dma, > | > > | > Could you run this through checkpatch, and fix any errors > if find? > | > It looks like you uses spaces for tabs in the code above, > maybe in > | > the other block too.. > | > > | > | It's because of his email client, it corrupted the whole patch like > | that. > > well, client seems to be > > | X-Mailer: git-send-email 1.5.2.2 > > more probably the text editor is the reason. > > -- Cyrill > yes, the editor 'vi' have configed "set expandtab". -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/atl1e/atl1e.h b/drivers/net/atl1e/atl1e.h index ba48220..d63be5c 100644 --- a/drivers/net/atl1e/atl1e.h +++ b/drivers/net/atl1e/atl1e.h @@ -377,10 +377,19 @@ struct atl1e_hw { */ struct atl1e_tx_buffer { struct sk_buff *skb; + unsigned long flags; +#define ATL1E_TX_PCIMAP_SINGLE 0x0001 +#define ATL1E_TX_PCIMAP_PAGE 0x0002 +#define ATL1E_TX_PCIMAP_TYPE_MASK 0x3 u16 length; dma_addr_t dma; }; +#define ATL1E_SET_PCIMAP_TYPE(tx_buff, type) do { \ + ((tx_buff)->flags) &= ~ATL1E_TX_PCIMAP_TYPE_MASK; \ + ((tx_buff)->flags) |= (type); \ + } while (0) + struct atl1e_rx_page { dma_addr_t dma; /* receive rage DMA address */ u8 *addr; /* receive rage virtual address */ diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c index 69b830f..955da73 100644 --- a/drivers/net/atl1e/atl1e_main.c +++ b/drivers/net/atl1e/atl1e_main.c @@ -635,7 +635,11 @@ static void atl1e_clean_tx_ring(struct atl1e_adapter *adapter) for (index = 0; index < ring_count; index++) { tx_buffer = &tx_ring->tx_buffer[index]; if (tx_buffer->dma) { - pci_unmap_page(pdev, tx_buffer->dma, + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) + pci_unmap_single(pdev, tx_buffer->dma, + tx_buffer->length, PCI_DMA_TODEVICE); + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) + pci_unmap_page(pdev, tx_buffer->dma, tx_buffer->length, PCI_DMA_TODEVICE); tx_buffer->dma = 0; } @@ -1220,7 +1224,11 @@ static bool atl1e_clean_tx_irq(struct atl1e_adapter *adapter) while (next_to_clean != hw_next_to_clean) { tx_buffer = &tx_ring->tx_buffer[next_to_clean]; if (tx_buffer->dma) { - pci_unmap_page(adapter->pdev, tx_buffer->dma, + if (tx_buffer->flags & ATL1E_TX_PCIMAP_SINGLE) + pci_unmap_single(adapter->pdev, tx_buffer->dma, + tx_buffer->length, PCI_DMA_TODEVICE); + else if (tx_buffer->flags & ATL1E_TX_PCIMAP_PAGE) + pci_unmap_page(adapter->pdev, tx_buffer->dma, tx_buffer->length, PCI_DMA_TODEVICE); tx_buffer->dma = 0; } @@ -1741,6 +1749,7 @@ static void atl1e_tx_map(struct atl1e_adapter *adapter, tx_buffer->length = map_len; tx_buffer->dma = pci_map_single(adapter->pdev, skb->data, hdr_len, PCI_DMA_TODEVICE); + ATL1E_SET_PCIMAP_TYPE(tx_buffer, ATL1E_TX_PCIMAP_SINGLE); mapped_len += map_len; use_tpd->buffer_addr = cpu_to_le64(tx_buffer->dma); use_tpd->word2 = (use_tpd->word2 & (~TPD_BUFLEN_MASK)) | @@ -1766,6 +1775,7 @@ static void atl1e_tx_map(struct atl1e_adapter *adapter, tx_buffer->dma = pci_map_single(adapter->pdev, skb->data + mapped_len, map_len, PCI_DMA_TODEVICE); + ATL1E_SET_PCIMAP_TYPE(tx_buffer, ATL1E_TX_PCIMAP_SINGLE); mapped_len += map_len; use_tpd->buffer_addr = cpu_to_le64(tx_buffer->dma); use_tpd->word2 = (use_tpd->word2 & (~TPD_BUFLEN_MASK)) | @@ -1801,6 +1811,7 @@ static void atl1e_tx_map(struct atl1e_adapter *adapter, (i * MAX_TX_BUF_LEN), tx_buffer->length, PCI_DMA_TODEVICE); + ATL1E_SET_PCIMAP_TYPE(tx_buffer, ATL1E_TX_PCIMAP_PAGE); use_tpd->buffer_addr = cpu_to_le64(tx_buffer->dma); use_tpd->word2 = (use_tpd->word2 & (~TPD_BUFLEN_MASK)) | ((cpu_to_le32(tx_buffer->length) &
[ 25.059969] WARNING: at lib/dma-debug.c:816 check_unmap+0x383/0x55c() [ 25.059976] Hardware name: 1000HE [ 25.059984] ATL1E 0000:03:00.0: DMA-API: device driver frees DMA memory with wrong function [device address=0x0000000036b92802] [size=90 bytes] [mapped as single] [unmapped as page] use the wrong API when free dma. So when map dma use a flag to demostrate whether it is 'pci_map_single' or 'pci_map_page'. When free the dma, check the flags to select the right APIs('pci_unmap_single' or 'pci_unmap_page'). Signed-off-by: Jie Yang <jie.yang@atheros.com> --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html