From: Stephen Hemminger on
On Wed, 20 Jan 2010 09:41:03 +0000
Jarek Poplawski <jarkao2(a)gmail.com> wrote:

> [ previously: Re: [PATCH] af_packet: Don't use skb after dev_queue_xmit() ]
> On Tue, Jan 19, 2010 at 05:10:13PM -0800, Stephen Hemminger wrote:
> > On Tue, 19 Jan 2010 20:01:10 -0500
> > Michael Breuer <mbreuer(a)majjas.com> wrote:
> >
> > > On 1/19/2010 5:45 PM, Jarek Poplawski wrote:
> > > > On Tue, Jan 19, 2010 at 03:06:01PM -0500, Michael Breuer wrote:
> > > >
> > > >> On 1/19/2010 2:59 PM, Jarek Poplawski wrote:
> > > >>
> > > >>> On Tue, Jan 19, 2010 at 10:47:27AM -0500, Michael Breuer wrote:
> > > >>> ...
> > > >>>
> > > >>>> Still get the warning... but now 60 bytes.
> > > >>>> Jan 19 10:43:50 mail kernel: ------------[ cut here ]------------
> > > >>>> Jan 19 10:43:50 mail kernel: WARNING: at lib/dma-debug.c:902
> ...
> > > That not only compiled, but it cleared the error as well. Additionally,
> > > I used to see a bit of a delay receiving the login prompt when first
> > > connecting to the box by ssh. That delay is gone with this patch. I'd
> > > guess that the warning wasn't quite as innocuous as I thought.
> > > Note: tested on 2.6.32.4. I'll leave this up for a bit before
> > > attempting to move back to head.
> >
> > Seems like an underlying bug in the DMA api. Maybe it just can't
> > handle operations on partial mapping.
> >
> > Other drivers with same problem:
> > bnx2, cassini, pcnet32, r8169, rrunner, skge, sungem, tg3,
>
> It seems using the same length (even without pci_unmap_len()) is
> crucial here, but I hope maintainers (added to CC) will take care.
>
> Btw, it's not tested yet, but it might affect CONFIG_DMAR problems.

The list of places to inspect is:

$ git grep -l sync_single_for_cpu $(git grep -l skb_copy_from_linear_data)
drivers/infiniband/ulp/ipoib/ipoib_cm.c
drivers/message/fusion/mptlan.c
drivers/net/b44.c
drivers/net/bnx2.c
drivers/net/cassini.c
drivers/net/chelsio/sge.c
drivers/net/cxgb3/sge.c
drivers/net/irda/vlsi_ir.c
drivers/net/myri_sbus.c
drivers/net/r8169.c
drivers/net/rrunner.c
drivers/net/skge.c
drivers/net/sky2.c
drivers/net/sungem.c
drivers/net/sunhme.c
drivers/net/tg3.c
drivers/net/tokenring/3c359.c
drivers/net/tokenring/olympic.c
drivers/net/tulip/de2104x.c
drivers/net/via-velocity.c
drivers/net/wireless/ipw2x00/ipw2100.c
drivers/net/wireless/ipw2x00/ipw2200.c
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Hemminger on
On Wed, 20 Jan 2010 09:41:03 +0000
Jarek Poplawski <jarkao2(a)gmail.com> wrote:

> It seems using the same length (even without pci_unmap_len()) is
> crucial here, but I hope maintainers (added to CC) will take care.
>
> Btw, it's not tested yet, but it might affect CONFIG_DMAR problems.
>
> Thanks,
> Jarek P.

Update Documentation/PCI-DMA-mapping as well please.

--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Stephen Hemminger on
On Wed, 20 Jan 2010 12:11:52 -0800
"Michael Chan" <mchan(a)broadcom.com> wrote:

>
> On Wed, 2010-01-20 at 10:03 -0800, Stephen Hemminger wrote:
> > On Wed, 20 Jan 2010 09:41:03 +0000
> > Jarek Poplawski <jarkao2(a)gmail.com> wrote:
> > > > Seems like an underlying bug in the DMA api. Maybe it just can't
> > > > handle operations on partial mapping.
> > > >
> > > > Other drivers with same problem:
> > > > bnx2, cassini, pcnet32, r8169, rrunner, skge, sungem, tg3,
> > >
> > > It seems using the same length (even without pci_unmap_len()) is
> > > crucial here, but I hope maintainers (added to CC) will take care.
> > >
>
> I'm still unsure how to do dma_sync properly in bnx2. In the current
> code, we always dma_sync_for_cpu a small portion of the SKB because the
> rx descriptor is at the beginning of the SKB. We get the packet length,
> for example, from the rx descriptor.
>
> If it's a big packet, we'll simply unmap the entire SKB buffer (with the
> beginning portion already dma_sync'ed). If the packet is smaller than
> what we dma_sync'ed, we'll just copy the data to a new SKB. We'll then
> dma_sync_for_device the portion of the original buffer and recycle the
> whole buffer back to the device for new packets.
>
> So, is it correct to just change the dma_sync length to the full length
> of the buffer? It doesn't sound right to me.

It looks like the size passed to sync_single has to match size of original
mapping.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jarek Poplawski on
On Wed, Jan 20, 2010 at 12:30:33PM -0800, Stephen Hemminger wrote:
> On Wed, 20 Jan 2010 12:11:52 -0800
> "Michael Chan" <mchan(a)broadcom.com> wrote:
>
> >
> > On Wed, 2010-01-20 at 10:03 -0800, Stephen Hemminger wrote:
> > > On Wed, 20 Jan 2010 09:41:03 +0000
> > > Jarek Poplawski <jarkao2(a)gmail.com> wrote:
> > > > > Seems like an underlying bug in the DMA api. Maybe it just can't
> > > > > handle operations on partial mapping.
> > > > >
> > > > > Other drivers with same problem:
> > > > > bnx2, cassini, pcnet32, r8169, rrunner, skge, sungem, tg3,
> > > >
> > > > It seems using the same length (even without pci_unmap_len()) is
> > > > crucial here, but I hope maintainers (added to CC) will take care.
> > > >
> >
> > I'm still unsure how to do dma_sync properly in bnx2. In the current
> > code, we always dma_sync_for_cpu a small portion of the SKB because the
> > rx descriptor is at the beginning of the SKB. We get the packet length,
> > for example, from the rx descriptor.
> >
> > If it's a big packet, we'll simply unmap the entire SKB buffer (with the
> > beginning portion already dma_sync'ed). If the packet is smaller than
> > what we dma_sync'ed, we'll just copy the data to a new SKB. We'll then
> > dma_sync_for_device the portion of the original buffer and recycle the
> > whole buffer back to the device for new packets.
> >
> > So, is it correct to just change the dma_sync length to the full length
> > of the buffer? It doesn't sound right to me.
>
> It looks like the size passed to sync_single has to match size of original
> mapping.

Yes, and it's mainly for lib/dma-debug (until it's not verified dmar
errors reported by Michael Breuer could be connected). So, I'm not
sure for now how serious this warning could be. On the other hand,
Documentation/(PCI-?)DMA-mapping, mentioned by Stephen, doesn't seem
to allow or use in examples the "size" different than mapped.

Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
> > Seems like an underlying bug in the DMA api. Maybe it just can't
> > handle operations on partial mapping.
> >
> > Other drivers with same problem:
> > bnx2, cassini, pcnet32, r8169, rrunner, skge, sungem, tg3,
>
> It seems using the same length (even without pci_unmap_len()) is
> crucial here, but I hope maintainers (added to CC) will take care.

The API needs fixing - if you've got a large mapping and you want to sync
part of it then we need to support that. Now it might well be that the
implementation on some braindead platform has to sync the entire thing,
and some implementations entire pages or cache lines.

You can't fix this in the drivers, they requested a service and they
don't have enough information nor is it their job to know about all the
platform specific rules.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/