sky2: Fix WARNING: at lib/dma-debug.c:902 check_sync
in [Kernel]
|
Prev: : bug fix, remove partial zero out
Next: Strange EHCI/OHCI IRQ and other PCI errors on boot with 2.6.32 (which 2.6.26 did not see) on Geode LX/CS5536.
From: David Miller on 20 Jan 2010 17:50 From: Stephen Hemminger <shemminger(a)linux-foundation.org> Date: Wed, 20 Jan 2010 12:30:33 -0800 > It looks like the size passed to sync_single has to match size of original > mapping. I think that's rediculious, and unreasonable. All implementations of these APIs where the sync matters have the ability to handle partial syncs correctly. -- 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 20 Jan 2010 18:00 On Wed, Jan 20, 2010 at 10:24:14PM +0000, Alan Cox 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. > > 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. Yes, the need to repeat some other values if there is a dedicated structure/pointer could be misleading. Btw, it seems to be a trivial overlooking since there is dma_sync_single_range() ready to use. 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: David Miller on 20 Jan 2010 18:00 From: Jarek Poplawski <jarkao2(a)gmail.com> Date: Wed, 20 Jan 2010 21:58:42 +0100 > 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. Someone find me even one actual implemenetation of the DMA APIs, not the debugging code, which can't handle a partial sync correctly, then we can talk. As the person who originally created the PCI DMA APIs I can tell you the intention was definitely to allow arbitrary SYNC lengths and there was no requirement that the length needed to be the same as that used in the mapping call. Nothing else makes sense. People sync when they have a partially used buffer and want to copy the data out then recycle the big original buffer back to the chip. I knew this when creating the PCI DMA APIs because I first tried to use them in network drivers and that's what so many of them do. Currently all we have is an overly anal DMA debugging layer check, and some speculation that it might or might not fix some real bug. -- 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: David Miller on 20 Jan 2010 18:00 From: Alan Cox <alan(a)lxorguk.ukuu.org.uk> Date: Wed, 20 Jan 2010 22:24:14 +0000 >> > 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. Absolutely and %100 agreed, the DMA debugging layer and documentation are both buggy. The intention from the beginning was always to allow partial SYNCs exactly for the reasons Alan and myself are mentioning here. -- 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: FUJITA Tomonori on 21 Jan 2010 10:30
On Wed, 20 Jan 2010 23:53:22 +0100 Jarek Poplawski <jarkao2(a)gmail.com> wrote: > On Wed, Jan 20, 2010 at 10:24:14PM +0000, Alan Cox 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. > > > > 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. > > Yes, the need to repeat some other values if there is a dedicated > structure/pointer could be misleading. Btw, it seems to be a trivial > overlooking since there is dma_sync_single_range() ready to use. Yeah, dma_sync_single_range() enables you to do a partial sync. But you must be really careful with a partial sync (as DMA-API.txt says). -- 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/ |