Re: [PATCH 09/18] dma-debug: add checking for map/unmap_page/single

From: FUJITA Tomonori
Date: Wed Mar 18 2009 - 21:41:10 EST


On Fri, 6 Mar 2009 14:30:20 +0100
Joerg Roedel <joerg.roedel@xxxxxxx> wrote:

> Impact: add debug callbacks for dma_{un}map_[page|single]
>
> Signed-off-by: Joerg Roedel <joerg.roedel@xxxxxxx>
> ---
> include/linux/dma-debug.h | 23 +++++++++++++++++++
> lib/dma-debug.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
> index 345d538..65f7352 100644
> --- a/include/linux/dma-debug.h
> +++ b/include/linux/dma-debug.h
> @@ -28,12 +28,35 @@ struct device;
>
> extern void dma_debug_init(u32 num_entries);
>
> +extern void debug_dma_map_page(struct device *dev, struct page *page,
> + size_t offset, size_t size,
> + int direction, dma_addr_t dma_addr,
> + bool map_single);
> +
> +extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, int direction, bool map_single);
> +
> +
> #else /* CONFIG_DMA_API_DEBUG */
>
> static inline void dma_debug_init(u32 num_entries)
> {
> }
>
> +static inline void debug_dma_map_page(struct device *dev, struct page *page,
> + size_t offset, size_t size,
> + int direction, dma_addr_t dma_addr,
> + bool map_single)
> +{
> +}
> +
> +static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, int direction,
> + bool map_single)
> +{
> +}
> +
> +
> #endif /* CONFIG_DMA_API_DEBUG */
>
> #endif /* __DMA_DEBUG_H */
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index d0cb47a..a2ed2b7 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -566,3 +566,56 @@ out:
>
> }
>
> +void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
> + size_t size, int direction, dma_addr_t dma_addr,
> + bool map_single)
> +{
> + struct dma_debug_entry *entry;
> +
> + if (unlikely(global_disable))
> + return;
> +
> + if (unlikely(dma_mapping_error(dev, dma_addr)))
> + return;
> +
> + entry = dma_entry_alloc();
> + if (!entry)
> + return;
> +
> + entry->dev = dev;
> + entry->type = dma_debug_page;
> + entry->paddr = page_to_phys(page) + offset;
> + entry->dev_addr = dma_addr;
> + entry->size = size;
> + entry->direction = direction;
> +
> + if (map_single) {
> + entry->type = dma_debug_single;
> + check_for_stack(dev, page_address(page) + offset);

Why you don't call check_for_stack() for dma_map_page()?

page_address(page) could be invalid with dma_map_page() so the check
can be pointless. However, you call check_for_stack() with dma_map_sg,
which the check can be pointless too with; I think that you call
check_for_stack() in an inconsistent way.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/