2009-09-16 06:22:56

by Jie Yang

[permalink] [raw]
Subject: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA


[ 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 <[email protected]>
---
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) &


2009-09-16 06:57:09

by David Miller

[permalink] [raw]
Subject: Re: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

From: <[email protected]>
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 <[email protected]>

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.

2009-09-16 09:09:57

by Daniel Walker

[permalink] [raw]
Subject: Re: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

On Wed, 2009-09-16 at 14:21 +0800, [email protected] 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

2009-09-16 09:37:06

by Jie Yang

[permalink] [raw]
Subject: RE: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

On Wednesday, September 16, 2009 5:11 PM
Daniel Walker <[email protected]> wrote:

> On Wed, 2009-09-16 at 14:21 +0800, [email protected] 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

2009-09-16 09:47:44

by David Miller

[permalink] [raw]
Subject: Re: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

From: Daniel Walker <[email protected]>
Date: Wed, 16 Sep 2009 02:10:51 -0700

> On Wed, 2009-09-16 at 14:21 +0800, [email protected] 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.

2009-09-16 09:51:39

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

[David Miller - Wed, Sep 16, 2009 at 02:47:59AM -0700]
| From: Daniel Walker <[email protected]>
| Date: Wed, 16 Sep 2009 02:10:51 -0700
|
| > On Wed, 2009-09-16 at 14:21 +0800, [email protected] 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

2009-09-16 09:54:40

by Jie Yang

[permalink] [raw]
Subject: RE: [Patch net-next]atl1e:fix 2.6.31-git4 -- ATL1E 0000:03:00.0: DMA-API: device driver frees DMA

On Wednesday, September 16, 2009 5:52 PM
Cyrill Gorcunov <[email protected]> wrote:

> [David Miller - Wed, Sep 16, 2009 at 02:47:59AM -0700]
> | From: Daniel Walker <[email protected]>
> | Date: Wed, 16 Sep 2009 02:10:51 -0700
> |
> | > On Wed, 2009-09-16 at 14:21 +0800, [email protected] 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".