| Message ID | 1231517970-20288-16-git-send-email-joerg.roedel@amd.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
* Joerg Roedel <joerg.roedel@amd.com> wrote: > @@ -105,6 +109,7 @@ dma_unmap_single(struct device *dev, dma_addr_t addr, size_t size, > struct dma_mapping_ops *ops = get_dma_ops(dev); > > BUG_ON(!valid_dma_direction(direction)); > + debug_unmap_single(dev, addr, size, direction); It all looks very nice, i've got one small namespace structure request: could you please name all the callbacks in a consistent way, so that they mirror the method they instrument - with a "debug_" prefix? I.e. the above one would be: debug_dma_unmap_single(). Ingo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Joerg Roedel <joerg.roedel@amd.com> wrote: > @@ -175,6 +189,8 @@ dma_sync_single_range_for_device(struct device *hwdev, dma_addr_t dma_handle, > struct dma_mapping_ops *ops = get_dma_ops(hwdev); > > BUG_ON(!valid_dma_direction(direction)); > + debug_sync_single_range_for_device(hwdev, dma_handle, offset, whitespace damage here. Ingo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 9 Jan 2009 17:19:29 +0100 Joerg Roedel <joerg.roedel@amd.com> wrote: > Impact: make use of DMA-API debugging code in x86 > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> > --- > arch/x86/Kconfig | 1 + > arch/x86/include/asm/dma-mapping.h | 30 ++++++++++++++++++++++++++---- > arch/x86/kernel/pci-dma.c | 5 +++++ > 3 files changed, 32 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 862adb9..68a806c 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -39,6 +39,7 @@ config X86 > select HAVE_GENERIC_DMA_COHERENT if X86_32 > select HAVE_EFFICIENT_UNALIGNED_ACCESS > select USER_STACKTRACE_SUPPORT > + select HAVE_DMA_API_DEBUG > > config ARCH_DEFCONFIG > string > diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h > index 4035357..939d5b3 100644 > --- a/arch/x86/include/asm/dma-mapping.h > +++ b/arch/x86/include/asm/dma-mapping.h > @@ -7,6 +7,7 @@ > */ > > #include <linux/scatterlist.h> > +#include <linux/dma-debug.h> > #include <asm/io.h> > #include <asm/swiotlb.h> > #include <asm-generic/dma-coherent.h> > @@ -93,9 +94,12 @@ dma_map_single(struct device *hwdev, void *ptr, size_t size, > int direction) > { > struct dma_mapping_ops *ops = get_dma_ops(hwdev); > + dma_addr_t addr; > > BUG_ON(!valid_dma_direction(direction)); > - return ops->map_single(hwdev, virt_to_phys(ptr), size, direction); > + addr = ops->map_single(hwdev, virt_to_phys(ptr), size, direction); > + debug_map_single(hwdev, ptr, size, direction, addr); > + return addr; What happens if ops->map_single fails? Seems that debug_map_single doesn't check the dma mapping fails. So it allocates a new entries and nobody frees it? Another problem is that what happens if ops->map_single succeeds but debug_map_single fails? Seems that it gives a false warning. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 11, 2009 at 03:25:47PM +0900, FUJITA Tomonori wrote: > On Fri, 9 Jan 2009 17:19:29 +0100 > Joerg Roedel <joerg.roedel@amd.com> wrote: > > > Impact: make use of DMA-API debugging code in x86 > > > > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> > > --- > > arch/x86/Kconfig | 1 + > > arch/x86/include/asm/dma-mapping.h | 30 ++++++++++++++++++++++++++---- > > arch/x86/kernel/pci-dma.c | 5 +++++ > > 3 files changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 862adb9..68a806c 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -39,6 +39,7 @@ config X86 > > select HAVE_GENERIC_DMA_COHERENT if X86_32 > > select HAVE_EFFICIENT_UNALIGNED_ACCESS > > select USER_STACKTRACE_SUPPORT > > + select HAVE_DMA_API_DEBUG > > > > config ARCH_DEFCONFIG > > string > > diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h > > index 4035357..939d5b3 100644 > > --- a/arch/x86/include/asm/dma-mapping.h > > +++ b/arch/x86/include/asm/dma-mapping.h > > @@ -7,6 +7,7 @@ > > */ > > > > #include <linux/scatterlist.h> > > +#include <linux/dma-debug.h> > > #include <asm/io.h> > > #include <asm/swiotlb.h> > > #include <asm-generic/dma-coherent.h> > > @@ -93,9 +94,12 @@ dma_map_single(struct device *hwdev, void *ptr, size_t size, > > int direction) > > { > > struct dma_mapping_ops *ops = get_dma_ops(hwdev); > > + dma_addr_t addr; > > > > BUG_ON(!valid_dma_direction(direction)); > > - return ops->map_single(hwdev, virt_to_phys(ptr), size, direction); > > + addr = ops->map_single(hwdev, virt_to_phys(ptr), size, direction); > > + debug_map_single(hwdev, ptr, size, direction, addr); > > + return addr; > > What happens if ops->map_single fails? > > Seems that debug_map_single doesn't check the dma mapping fails. So it > allocates a new entries and nobody frees it? Ah true. The debug_map_single function has to check for dma_mapping_error. Same is true for debug_map_sg. > Another problem is that what happens if ops->map_single succeeds but > debug_map_single fails? Seems that it gives a false warning. No, this can not happen. If the code fails it disables itself so no more warnings will be printed. Joerg -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 862adb9..68a806c 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -39,6 +39,7 @@ config X86 select HAVE_GENERIC_DMA_COHERENT if X86_32 select HAVE_EFFICIENT_UNALIGNED_ACCESS select USER_STACKTRACE_SUPPORT + select HAVE_DMA_API_DEBUG config ARCH_DEFCONFIG string diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h index 4035357..939d5b3 100644 --- a/arch/x86/include/asm/dma-mapping.h +++ b/arch/x86/include/asm/dma-mapping.h @@ -7,6 +7,7 @@ */ #include <linux/scatterlist.h> +#include <linux/dma-debug.h> #include <asm/io.h> #include <asm/swiotlb.h> #include <asm-generic/dma-coherent.h> @@ -93,9 +94,12 @@ dma_map_single(struct device *hwdev, void *ptr, size_t size, int direction) { struct dma_mapping_ops *ops = get_dma_ops(hwdev); + dma_addr_t addr; BUG_ON(!valid_dma_direction(direction)); - return ops->map_single(hwdev, virt_to_phys(ptr), size, direction); + addr = ops->map_single(hwdev, virt_to_phys(ptr), size, direction); + debug_map_single(hwdev, ptr, size, direction, addr); + return addr; } static inline void @@ -105,6 +109,7 @@ dma_unmap_single(struct device *dev, dma_addr_t addr, size_t size, struct dma_mapping_ops *ops = get_dma_ops(dev); BUG_ON(!valid_dma_direction(direction)); + debug_unmap_single(dev, addr, size, direction); if (ops->unmap_single) ops->unmap_single(dev, addr, size, direction); } @@ -114,9 +119,13 @@ dma_map_sg(struct device *hwdev, struct scatterlist *sg, int nents, int direction) { struct dma_mapping_ops *ops = get_dma_ops(hwdev); + int ret; BUG_ON(!valid_dma_direction(direction)); - return ops->map_sg(hwdev, sg, nents, direction); + ret = ops->map_sg(hwdev, sg, nents, direction); + debug_map_sg(hwdev, sg, ret, direction); + + return ret; } static inline void @@ -126,6 +135,7 @@ dma_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_unmap_sg(hwdev, sg, nents, direction); if (ops->unmap_sg) ops->unmap_sg(hwdev, sg, nents, direction); } @@ -137,6 +147,7 @@ dma_sync_single_for_cpu(struct device *hwdev, dma_addr_t dma_handle, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_single_for_cpu(hwdev, dma_handle, size, direction); if (ops->sync_single_for_cpu) ops->sync_single_for_cpu(hwdev, dma_handle, size, direction); flush_write_buffers(); @@ -149,6 +160,7 @@ dma_sync_single_for_device(struct device *hwdev, dma_addr_t dma_handle, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_single_for_device(hwdev, dma_handle, size, direction); if (ops->sync_single_for_device) ops->sync_single_for_device(hwdev, dma_handle, size, direction); flush_write_buffers(); @@ -161,6 +173,8 @@ dma_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dma_handle, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_single_range_for_cpu(hwdev, dma_handle, offset, size, + direction); if (ops->sync_single_range_for_cpu) ops->sync_single_range_for_cpu(hwdev, dma_handle, offset, size, direction); @@ -175,6 +189,8 @@ dma_sync_single_range_for_device(struct device *hwdev, dma_addr_t dma_handle, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_single_range_for_device(hwdev, dma_handle, offset, + size, direction); if (ops->sync_single_range_for_device) ops->sync_single_range_for_device(hwdev, dma_handle, offset, size, direction); @@ -188,6 +204,7 @@ dma_sync_sg_for_cpu(struct device *hwdev, struct scatterlist *sg, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_sg_for_cpu(hwdev, sg, nelems, direction); if (ops->sync_sg_for_cpu) ops->sync_sg_for_cpu(hwdev, sg, nelems, direction); flush_write_buffers(); @@ -200,6 +217,7 @@ dma_sync_sg_for_device(struct device *hwdev, struct scatterlist *sg, struct dma_mapping_ops *ops = get_dma_ops(hwdev); BUG_ON(!valid_dma_direction(direction)); + debug_sync_sg_for_device(hwdev, sg, nelems, direction); if (ops->sync_sg_for_device) ops->sync_sg_for_device(hwdev, sg, nelems, direction); @@ -267,7 +285,7 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t gfp) { struct dma_mapping_ops *ops = get_dma_ops(dev); - void *memory; + void *memory, *addr; gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32); @@ -285,8 +303,11 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle, if (!ops->alloc_coherent) return NULL; - return ops->alloc_coherent(dev, size, dma_handle, + addr = ops->alloc_coherent(dev, size, dma_handle, dma_alloc_coherent_gfp_flags(dev, gfp)); + debug_alloc_coherent(dev, size, *dma_handle, addr); + + return addr; } static inline void dma_free_coherent(struct device *dev, size_t size, @@ -299,6 +320,7 @@ static inline void dma_free_coherent(struct device *dev, size_t size, if (dma_release_from_coherent(dev, get_order(size), vaddr)) return; + debug_free_coherent(dev, size, vaddr, bus); if (ops->free_coherent) ops->free_coherent(dev, size, vaddr, bus); } diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c index b254285..c8efbcc 100644 --- a/arch/x86/kernel/pci-dma.c +++ b/arch/x86/kernel/pci-dma.c @@ -44,6 +44,9 @@ struct device x86_dma_fallback_dev = { }; EXPORT_SYMBOL(x86_dma_fallback_dev); +/* Number of entries preallocated for DMA-API debugging */ +#define PREALLOC_ENTRIES 8192 /* needs 512kb */ + int dma_set_mask(struct device *dev, u64 mask) { if (!dev->dma_mask || !dma_supported(dev, mask)) @@ -265,6 +268,8 @@ EXPORT_SYMBOL(dma_supported); static int __init pci_iommu_init(void) { + dma_debug_init(PREALLOC_ENTRIES); + calgary_iommu_init(); intel_iommu_init();
Impact: make use of DMA-API debugging code in x86 Signed-off-by: Joerg Roedel <joerg.roedel@amd.com> --- arch/x86/Kconfig | 1 + arch/x86/include/asm/dma-mapping.h | 30 ++++++++++++++++++++++++++---- arch/x86/kernel/pci-dma.c | 5 +++++ 3 files changed, 32 insertions(+), 4 deletions(-)