* 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).