Skip to contentSkip to conversation

dma_mapping_error() interfaces

DISCUSSION Shuah Khan 2 participants 3 messages Oct 25, 2012 – Nov 1, 2012

lore
Share & tools
lore

Shuah Khan

author
#0 lore raw

I am working on changing dma_mapping_error() interfaces to add call to debug_dma_mapping_error() interface. I found several dma_mapping_error() defines in various arch specific dma-mapping.h files.

Majority of these arch specific defines look identical - simply

static inline int dma_mapping_error(struct device *dev, dma_ addr_t dma_addr)

{
       struct dma_map_ops *dma_ops = get_dma_ops(dev);

       if (dma_ops->mapping_error)
                return dma_ops->mapping_error(dev, dma_addr);
}

Maybe a generic dma_mapping_error() could be added to augment the existing dma_map_page() and dma_map_single() generic interfaces in include/asm-generic/dma-mapping-common.h (Future work)

For the most part, architecture specific dma_map_page()/dma_map_single(), dma_unmap_page() interfaces are implemented as a pair, however arch/m68k/include/asm/dma-mapping.h is an exception. In this case generic dma_map_page()/dma_map_single() are used, and arch specific dma_unmap_page() is defined. Couple of issues, unmap routines do nothing. Maybe just fine for this architecture. However, when dma_debug is enabled, debug is lopsided and debug entries don't get releases.

For the present here is what I am planning to do:

Change dma_mapping_error() defines to add debug_dma_mapping_error() on architectures that use the generic dma_map_page() and dma_map_single() interfaces from include/asm-generic/dma-mapping-common.h to debug inconsistency between invoking dma-debug at mapping time and missing debug at error check time. Inconsistency will result in false warnings. The following fall into this category.

arch/alpha/include/asm/dma-mapping.h
arch/arm/include/asm/dma-mapping.h
arch/arm64/include/asm/dma-mapping.h
arch/c6x/include/asm/dma-mapping.h
arch/frv/include/asm/dma-mapping.h
arch/hexagon/include/asm/dma-mapping.h
arch/ia64/include/asm/dma-mapping.h
arch/m68k/include/asm/dma-mapping.h
arch/microblaze/include/asm/dma-mapping.h
arch/mips/include/asm/dma-mapping.h
arch/powerpc/include/asm/dma-mapping.h
arch/sh/include/asm/dma-mapping.h
arch/sparc/include/asm/dma-mapping.h

Leave dma_mapping_error() defines as is in arch header files that also define dma_map_page() and dma_map_single() and not call debug_dma_map_page() interface. The following fall into this category.

arch/avr32/include/asm/dma-mapping.h
arch/blackfin/include/asm/dma-mapping.h
arch/cris/include/asm/dma-mapping.h
arch/mn10300/include/asm/dma-mapping.h
arch/parisc/include/asm/dma-mapping.h
arch/xtensa/include/asm/dma-mapping.h

- There is this inconsistency now with the debug_dma_mapping_error() interface in linux-next. If this is unacceptable, I will send a patch that marks dma debug entries checked for mapping errors in the interim (an easy change to initialize the flag to have been checked) while I get the existing dma_mapping_error() interface patches done.

Comments, concerns.

Thanks,
-- Shuah

-

#1 lore raw
Quoted 19 lines · Shuah Khan

I am working on changing dma_mapping_error() interfaces to add call to debug_dma_mapping_error() interface. I found several dma_mapping_error() defines in various arch specific dma-mapping.h files.

Majority of these arch specific defines look identical - simply

static inline int dma_mapping_error(struct device *dev, dma_ addr_t dma_addr)

{
       struct dma_map_ops *dma_ops = get_dma_ops(dev);

       if (dma_ops->mapping_error)
                return dma_ops->mapping_error(dev, dma_addr);
}

Maybe a generic dma_mapping_error() could be added to augment the existing dma_map_page() and dma_map_single() generic interfaces in include/asm-generic/dma-mapping-common.h (Future work)

Quoted 6 lines

For the most part, architecture specific dma_map_page()/dma_map_single(), dma_unmap_page() interfaces are implemented as a pair, however arch/m68k/include/asm/dma-mapping.h is an exception. In this case generic dma_map_page()/dma_map_single() are used, and arch specific dma_unmap_page() is defined. Couple of issues, unmap routines do nothing. Maybe just fine for this architecture.

Could you CC the maintainer of said architecture and find out? Perhaps this was intended.. or just a bug.

Quoted 12 lines

However, when dma_debug is enabled, debug is lopsided and debug entries don't get releases.

For the present here is what I am planning to do:

Change dma_mapping_error() defines to add debug_dma_mapping_error() on architectures that use the generic dma_map_page() and dma_map_single() interfaces from include/asm-generic/dma-mapping-common.h to debug inconsistency between invoking dma-debug at mapping time and missing debug at error check time. Inconsistency will result in false warnings. The following fall into this category.

Quoted 13 lines

arch/alpha/include/asm/dma-mapping.h
arch/arm/include/asm/dma-mapping.h
arch/arm64/include/asm/dma-mapping.h
arch/c6x/include/asm/dma-mapping.h
arch/frv/include/asm/dma-mapping.h
arch/hexagon/include/asm/dma-mapping.h
arch/ia64/include/asm/dma-mapping.h
arch/m68k/include/asm/dma-mapping.h
arch/microblaze/include/asm/dma-mapping.h
arch/mips/include/asm/dma-mapping.h
arch/powerpc/include/asm/dma-mapping.h
arch/sh/include/asm/dma-mapping.h
arch/sparc/include/asm/dma-mapping.h

Sounds good.

Quoted 11 lines

Leave dma_mapping_error() defines as is in arch header files that also define dma_map_page() and dma_map_single() and not call debug_dma_map_page() interface. The following fall into this category.

arch/avr32/include/asm/dma-mapping.h
arch/blackfin/include/asm/dma-mapping.h
arch/cris/include/asm/dma-mapping.h
arch/mn10300/include/asm/dma-mapping.h
arch/parisc/include/asm/dma-mapping.h
arch/xtensa/include/asm/dma-mapping.h

Any reason to skip those?

Quoted 16 lines

- There is this inconsistency now with the debug_dma_mapping_error() interface in linux-next. If this is unacceptable, I will send a patch that marks dma debug entries checked for mapping errors in the interim (an easy change to initialize the flag to have been checked) while I get the existing dma_mapping_error() interface patches done.

Comments, concerns.

Thanks,
-- Shuah

-

author
#2 lore raw
Quoted 27 lines · Konrad Rzeszutek Wilk
Quoted 19 lines · Shuah Khan

I am working on changing dma_mapping_error() interfaces to add call to debug_dma_mapping_error() interface. I found several dma_mapping_error() defines in various arch specific dma-mapping.h files.

Majority of these arch specific defines look identical - simply

static inline int dma_mapping_error(struct device *dev, dma_ addr_t dma_addr)

{
       struct dma_map_ops *dma_ops = get_dma_ops(dev);

       if (dma_ops->mapping_error)
                return dma_ops->mapping_error(dev, dma_addr);
}

Maybe a generic dma_mapping_error() could be added to augment the existing dma_map_page() and dma_map_single() generic interfaces in include/asm-generic/dma-mapping-common.h (Future work)

Quoted 6 lines

For the most part, architecture specific dma_map_page()/dma_map_single(), dma_unmap_page() interfaces are implemented as a pair, however arch/m68k/include/asm/dma-mapping.h is an exception. In this case generic dma_map_page()/dma_map_single() are used, and arch specific dma_unmap_page() is defined. Couple of issues, unmap routines do nothing. Maybe just fine for this architecture.

Quoted 2 lines

Could you CC the maintainer of said architecture and find out? Perhaps this was intended.. or just a bug.

Did more looking into this. I was wrong and it does defines arch specific dma_map_page(), dma_map_single(), and dma_mapping_error() interfaces. None of these call dma-debug at this time and there is no need to change dma_mapping_error() to call debug_dma_mapping_error().

Quoted 28 lines
Quoted 14 lines

However, when dma_debug is enabled, debug is lopsided and debug entries don't get releases.

For the present here is what I am planning to do:

Change dma_mapping_error() defines to add debug_dma_mapping_error() on architectures that use the generic dma_map_page() and dma_map_single() interfaces from include/asm-generic/dma-mapping-common.h to debug inconsistency between invoking dma-debug at mapping time and missing debug at error check time. Inconsistency will result in false warnings. The following fall into this category.

Quoted 13 lines

arch/alpha/include/asm/dma-mapping.h
arch/arm/include/asm/dma-mapping.h
arch/arm64/include/asm/dma-mapping.h
arch/c6x/include/asm/dma-mapping.h
arch/frv/include/asm/dma-mapping.h
arch/hexagon/include/asm/dma-mapping.h
arch/ia64/include/asm/dma-mapping.h
arch/m68k/include/asm/dma-mapping.h
arch/microblaze/include/asm/dma-mapping.h
arch/mips/include/asm/dma-mapping.h
arch/powerpc/include/asm/dma-mapping.h
arch/sh/include/asm/dma-mapping.h
arch/sparc/include/asm/dma-mapping.h

Quoted 1 line

Sounds good.

I did send arch patches last week tagging them for linux-next. I should follow-up to see what is happening to them.

Quoted 5 lines
Quoted 5 lines

Leave dma_mapping_error() defines as is in arch header files that also define dma_map_page() and dma_map_single() and not call debug_dma_map_page() interface. The following fall into this category.

Quoted 8 lines
Quoted 6 lines

arch/avr32/include/asm/dma-mapping.h
arch/blackfin/include/asm/dma-mapping.h
arch/cris/include/asm/dma-mapping.h
arch/mn10300/include/asm/dma-mapping.h
arch/parisc/include/asm/dma-mapping.h
arch/xtensa/include/asm/dma-mapping.h

Any reason to skip those?

All of these define architecture specific dma interfaces and none of them use dma debug at this time. Maybe something that could be enhanced, however there is nothing to fix based on the change I made to add debug_dma_mapping_error() to the dma-debug.

-- Shuah