* 8139cp dma-debug warning.
@ 2009-08-06 21:57 Dave Jones
2009-08-12 17:13 ` Dave Jones
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2009-08-06 21:57 UTC (permalink / raw)
To: netdev
I'm chasing yet another dma-debug warning where we're unmapping a different
size to what we mapped.
> WARNING: at lib/dma-debug.c:803 check_unmap+0x1f5/0x509() (Not tainted)
> Hardware name:
> 8139cp 0000:00:03.0: DMA-API: device driver frees DMA memory with different
> size [device address=0x000000001e9f8852] [map size=1536 bytes] [unmap size=1538
> bytes]
> Modules linked in: ipv6 dm_multipath uinput joydev 8139too virtio_balloon
> 8139cp mii i2c_piix4 virtio_pci i2c_core floppy squashfs pata_acpi ata_generic
> [last unloaded: scsi_wait_scan]
> Pid: 0, comm: swapper Not tainted 2.6.31-0.125.rc5.git2.fc12.i686 #1
> Call Trace:
> [<c0444408>] warn_slowpath_common+0x7b/0xa3
> [<c0606746>] ? check_unmap+0x1f5/0x509
> [<c0444499>] warn_slowpath_fmt+0x34/0x48
> [<c0606746>] check_unmap+0x1f5/0x509
> [<c04ecb43>] ? check_valid_pointer+0x2c/0x6c
> [<c0606c00>] debug_dma_unmap_page+0x62/0x7b
> [<e0dc80be>] dma_unmap_single_attrs.clone.2+0x5a/0x75 [8139cp]
> [<e0dc8220>] cp_rx_poll+0x147/0x301 [8139cp]
> [<c077e5d4>] net_rx_action+0xa7/0x1d3
> [<c044accf>] __do_softirq+0xc8/0x192
> [<c044ade2>] do_softirq+0x49/0x7f
> [<c044af36>] irq_exit+0x48/0x8c
> [<c041c302>] smp_apic_timer_interrupt+0x7a/0x99
> [<c0404416>] apic_timer_interrupt+0x36/0x3c
> [<c0425e98>] ? native_safe_halt+0xa/0xc
> [<c040ad6f>] default_idle+0x55/0x98
> [<c046e44d>] ? trace_hardirqs_off+0x19/0x2c
> [<c04029e3>] cpu_idle+0xac/0xcd
> [<c08080ee>] rest_init+0x66/0x79
> [<c0a72ae0>] start_kernel+0x36f/0x385
> [<c0a7207e>] __init_begin+0x7e/0x96
> ---[ end trace f3c3298e5df24f15 ]---
> Mapped at:
> [<c0606f6b>] debug_dma_map_page+0x6b/0x13b
> [<e0dc7c22>] dma_map_single_attrs.clone.1+0x78/0x93 [8139cp]
> [<e0dc86d0>] cp_init_rings+0xaa/0x12c [8139cp]
> [<e0dc87d6>] cp_open+0x84/0x154 [8139cp]
> [<c077ff2b>] dev_open+0x99/0xe4
Looking at 8139cp.c I see this code in cp_rx_poll ..
552 buflen = cp->rx_buf_sz + NET_IP_ALIGN;
553 new_skb = netdev_alloc_skb(dev, buflen);
554 if (!new_skb) {
555 dev->stats.rx_dropped++;
556 goto rx_next;
557 }
558
559 skb_reserve(new_skb, NET_IP_ALIGN);
Aren't we padding the alignment twice here? or am I missing something?
The warning was a difference of two bytes, which is NET_IP_ALIGN,
so I'm wondering if that skb_reserve line needs to be nuked ?
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 8139cp dma-debug warning.
2009-08-06 21:57 8139cp dma-debug warning Dave Jones
@ 2009-08-12 17:13 ` Dave Jones
2009-08-13 5:20 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2009-08-12 17:13 UTC (permalink / raw)
To: netdev
On Thu, Aug 06, 2009 at 05:57:02PM -0400, Dave Jones wrote:
> I'm chasing yet another dma-debug warning where we're unmapping a different
> size to what we mapped.
>
> > WARNING: at lib/dma-debug.c:803 check_unmap+0x1f5/0x509() (Not tainted)
> > Hardware name:
> > 8139cp 0000:00:03.0: DMA-API: device driver frees DMA memory with different
> > size [device address=0x000000001e9f8852] [map size=1536 bytes] [unmap size=1538
> > bytes]
> > Modules linked in: ipv6 dm_multipath uinput joydev 8139too virtio_balloon
> > 8139cp mii i2c_piix4 virtio_pci i2c_core floppy squashfs pata_acpi ata_generic
> > [last unloaded: scsi_wait_scan]
> > Pid: 0, comm: swapper Not tainted 2.6.31-0.125.rc5.git2.fc12.i686 #1
> > Call Trace:
> > [<c0444408>] warn_slowpath_common+0x7b/0xa3
> > [<c0606746>] ? check_unmap+0x1f5/0x509
> > [<c0444499>] warn_slowpath_fmt+0x34/0x48
> > [<c0606746>] check_unmap+0x1f5/0x509
> > [<c04ecb43>] ? check_valid_pointer+0x2c/0x6c
> > [<c0606c00>] debug_dma_unmap_page+0x62/0x7b
> > [<e0dc80be>] dma_unmap_single_attrs.clone.2+0x5a/0x75 [8139cp]
> > [<e0dc8220>] cp_rx_poll+0x147/0x301 [8139cp]
> > [<c077e5d4>] net_rx_action+0xa7/0x1d3
> > [<c044accf>] __do_softirq+0xc8/0x192
> > [<c044ade2>] do_softirq+0x49/0x7f
> > [<c044af36>] irq_exit+0x48/0x8c
> > [<c041c302>] smp_apic_timer_interrupt+0x7a/0x99
> > [<c0404416>] apic_timer_interrupt+0x36/0x3c
> > [<c0425e98>] ? native_safe_halt+0xa/0xc
> > [<c040ad6f>] default_idle+0x55/0x98
> > [<c046e44d>] ? trace_hardirqs_off+0x19/0x2c
> > [<c04029e3>] cpu_idle+0xac/0xcd
> > [<c08080ee>] rest_init+0x66/0x79
> > [<c0a72ae0>] start_kernel+0x36f/0x385
> > [<c0a7207e>] __init_begin+0x7e/0x96
> > ---[ end trace f3c3298e5df24f15 ]---
> > Mapped at:
> > [<c0606f6b>] debug_dma_map_page+0x6b/0x13b
> > [<e0dc7c22>] dma_map_single_attrs.clone.1+0x78/0x93 [8139cp]
> > [<e0dc86d0>] cp_init_rings+0xaa/0x12c [8139cp]
> > [<e0dc87d6>] cp_open+0x84/0x154 [8139cp]
> > [<c077ff2b>] dev_open+0x99/0xe4
>
> Looking at 8139cp.c I see this code in cp_rx_poll ..
>
> 552 buflen = cp->rx_buf_sz + NET_IP_ALIGN;
> 553 new_skb = netdev_alloc_skb(dev, buflen);
> 554 if (!new_skb) {
> 555 dev->stats.rx_dropped++;
> 556 goto rx_next;
> 557 }
> 558
> 559 skb_reserve(new_skb, NET_IP_ALIGN);
>
>
> Aren't we padding the alignment twice here? or am I missing something?
> The warning was a difference of two bytes, which is NET_IP_ALIGN,
> so I'm wondering if that skb_reserve line needs to be nuked ?
There's another instance of the same further in the file.
Does this look right?
Dave
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 07919d0..065c9c3 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -556,8 +556,6 @@ rx_status_loop:
goto rx_next;
}
- skb_reserve(new_skb, NET_IP_ALIGN);
-
dma_unmap_single(&cp->pdev->dev, mapping,
buflen, PCI_DMA_FROMDEVICE);
@@ -1061,8 +1059,6 @@ static int cp_refill_rx(struct cp_private *cp)
if (!skb)
goto err_out;
- skb_reserve(skb, NET_IP_ALIGN);
-
mapping = dma_map_single(&cp->pdev->dev, skb->data,
cp->rx_buf_sz, PCI_DMA_FROMDEVICE);
cp->rx_skb[i] = skb;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: 8139cp dma-debug warning.
2009-08-12 17:13 ` Dave Jones
@ 2009-08-13 5:20 ` David Miller
2009-08-13 13:45 ` Dave Jones
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2009-08-13 5:20 UTC (permalink / raw)
To: davej; +Cc: netdev
From: Dave Jones <davej@redhat.com>
Date: Wed, 12 Aug 2009 13:13:33 -0400
> There's another instance of the same further in the file.
>
> Does this look right?
Dave, Francois posted the following fix to lkml (he forgot
to CC: netdev and the people reporting this bug, oh well)
Did you see it? It should fix the bug.
8139cp: balance dma_map_single vs dma_unmap_single pair
The driver always:
1. allocate cp->rx_buf_sz + NET_IP_ALIGN
2. map cp->rx_buf_sz
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/8139cp.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 50efde1..d0dbbf3 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -515,7 +515,7 @@ rx_status_loop:
dma_addr_t mapping;
struct sk_buff *skb, *new_skb;
struct cp_desc *desc;
- unsigned buflen;
+ const unsigned buflen = cp->rx_buf_sz;
skb = cp->rx_skb[rx_tail];
BUG_ON(!skb);
@@ -549,8 +549,7 @@ rx_status_loop:
pr_debug("%s: rx slot %d status 0x%x len %d\n",
dev->name, rx_tail, status, len);
- buflen = cp->rx_buf_sz + NET_IP_ALIGN;
- new_skb = netdev_alloc_skb(dev, buflen);
+ new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
if (!new_skb) {
dev->stats.rx_dropped++;
goto rx_next;
--
1.6.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: 8139cp dma-debug warning.
2009-08-13 5:20 ` David Miller
@ 2009-08-13 13:45 ` Dave Jones
2009-08-13 19:23 ` Francois Romieu
0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2009-08-13 13:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Francois Romieu
On Wed, Aug 12, 2009 at 10:20:46PM -0700, David Miller wrote:
> From: Dave Jones <davej@redhat.com>
> Date: Wed, 12 Aug 2009 13:13:33 -0400
>
> > There's another instance of the same further in the file.
> >
> > Does this look right?
>
> Dave, Francois posted the following fix to lkml (he forgot
> to CC: netdev and the people reporting this bug, oh well)
Ah, I missed it (disk crashed on friday, so I've been backlogged
since then). Thanks for the forward.
> Did you see it? It should fix the bug.
>
> 8139cp: balance dma_map_single vs dma_unmap_single pair
>
> The driver always:
> 1. allocate cp->rx_buf_sz + NET_IP_ALIGN
> 2. map cp->rx_buf_sz
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/8139cp.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
> index 50efde1..d0dbbf3 100644
> --- a/drivers/net/8139cp.c
> +++ b/drivers/net/8139cp.c
> @@ -515,7 +515,7 @@ rx_status_loop:
> dma_addr_t mapping;
> struct sk_buff *skb, *new_skb;
> struct cp_desc *desc;
> - unsigned buflen;
> + const unsigned buflen = cp->rx_buf_sz;
>
> skb = cp->rx_skb[rx_tail];
> BUG_ON(!skb);
> @@ -549,8 +549,7 @@ rx_status_loop:
> pr_debug("%s: rx slot %d status 0x%x len %d\n",
> dev->name, rx_tail, status, len);
>
> - buflen = cp->rx_buf_sz + NET_IP_ALIGN;
> - new_skb = netdev_alloc_skb(dev, buflen);
> + new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
> if (!new_skb) {
> dev->stats.rx_dropped++;
> goto rx_next;
Below this, we're still doing an skb_reserve(NET_IP_ALIGN) on new_skb.
Although the mapping is now constantly sized, aren't we still wastefully
bumping the data/tail of the skb twice ?
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 8139cp dma-debug warning.
2009-08-13 13:45 ` Dave Jones
@ 2009-08-13 19:23 ` Francois Romieu
2009-08-13 19:28 ` Dave Jones
0 siblings, 1 reply; 6+ messages in thread
From: Francois Romieu @ 2009-08-13 19:23 UTC (permalink / raw)
To: Dave Jones; +Cc: David Miller, netdev
Dave Jones <davej@redhat.com> :
[...]
> > @@ -549,8 +549,7 @@ rx_status_loop:
> > pr_debug("%s: rx slot %d status 0x%x len %d\n",
> > dev->name, rx_tail, status, len);
> >
> > - buflen = cp->rx_buf_sz + NET_IP_ALIGN;
> > - new_skb = netdev_alloc_skb(dev, buflen);
> > + new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
> > if (!new_skb) {
> > dev->stats.rx_dropped++;
> > goto rx_next;
>
> Below this, we're still doing an skb_reserve(NET_IP_ALIGN) on new_skb.
> Although the mapping is now constantly sized, aren't we still wastefully
> bumping the data/tail of the skb twice ?
$ grep -n NET_IP_ALIGN drivers/net/8139cp.c
552: new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
558: skb_reserve(new_skb, NET_IP_ALIGN);
1059: skb = netdev_alloc_skb(dev, cp->rx_buf_sz + NET_IP_ALIGN);
1063: skb_reserve(skb, NET_IP_ALIGN);
I do not get it : netdev_alloc_skb allocates but it does not "bump" as
skb_reserve does (and skb_reserve does not allocate). Where would the
double bump come from ?
The mapping is constantly sized but we want the ethernet header following
IP data to be evenly aligned and thus the mapping to be oddly aligned.
--
Ueimor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 8139cp dma-debug warning.
2009-08-13 19:23 ` Francois Romieu
@ 2009-08-13 19:28 ` Dave Jones
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jones @ 2009-08-13 19:28 UTC (permalink / raw)
To: Francois Romieu; +Cc: David Miller, netdev
On Thu, Aug 13, 2009 at 09:23:26PM +0200, Francois Romieu wrote:
> > Below this, we're still doing an skb_reserve(NET_IP_ALIGN) on new_skb.
> > Although the mapping is now constantly sized, aren't we still wastefully
> > bumping the data/tail of the skb twice ?
>
> $ grep -n NET_IP_ALIGN drivers/net/8139cp.c
> 552: new_skb = netdev_alloc_skb(dev, buflen + NET_IP_ALIGN);
> 558: skb_reserve(new_skb, NET_IP_ALIGN);
> 1059: skb = netdev_alloc_skb(dev, cp->rx_buf_sz + NET_IP_ALIGN);
> 1063: skb_reserve(skb, NET_IP_ALIGN);
>
> I do not get it : netdev_alloc_skb allocates but it does not "bump" as
> skb_reserve does (and skb_reserve does not allocate). Where would the
> double bump come from ?
>
> The mapping is constantly sized but we want the ethernet header following
> IP data to be evenly aligned and thus the mapping to be oddly aligned.
Ah, now I get it. Thanks for the explanation :)
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-08-13 19:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-06 21:57 8139cp dma-debug warning Dave Jones
2009-08-12 17:13 ` Dave Jones
2009-08-13 5:20 ` David Miller
2009-08-13 13:45 ` Dave Jones
2009-08-13 19:23 ` Francois Romieu
2009-08-13 19:28 ` Dave Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).