lkml.org 
[lkml]   [2012]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: dma mapping error check analysis
On Fri, Aug 10, 2012 at 04:46:42PM -0600, Shuah Khan wrote:
> I analyzed current calls to dma_map_single() and dma_map_page() in the kernel
> to see if dma mapping errors are checked after mapping routines return.
>
> Reference linux-next August 6 2012.
>
> This analysis stemmed from the discussion on my patch that disables swiotlb
> overflow as a first step towards removing the support all together. Please
> refer to thread below:
>
> https://lkml.org/lkml/2012/7/24/391
>
> The goal of this analysis is to find drivers that don't currently check dma
> mapping errors and fix them. I did a grep for dma_map_single() and
> dma_map_page() and looked at the code that calls these routines. I classified
> the results of dma mapping error check status as follows:
>
> Broken:
> 1. No error checks
> 2. Partial checks - In that source file, not all calls are followed by checks.
> 3. Checks dma mapping errors, doesn't unmap already mapped pages when mapping
> error occurs in the middle of a multiple mapping attempt.
>
> The first two categories are classified as broken and need fixing.
>
> The third one needs fixing, since it leaves dangling mapped pages, and holds
> on to them which is equivalent to memory leak. Some drivers release all mapped
> pages when the device closes, but others don't. Not doing unmap might be
> harmless on some architectures going by the comments I found in some source
> files.
>
> Good:
> 1. Checks dma mapping errors and unmaps already mapped pages when mapping
> error occurs in the middle of a multiple mapping attempt.
> 2. Checks dma mapping errors without unlikely()
> 3. Checks dma mapping errors with unlikely()
>
> I lumped the above three cases as good cases. Using unlikely() is icing on the
> cake, and something we need to be concerned about compared to other problems in
> this area.
>
> - dmap_map_single() - results
> No error checks - 195 (46%)
> Partial checks - 46 (11%)
> Doesn't unmap: 26 (6%)
> Good: 147 (35%)
>
> - dma_map_page() - results
> No error checks: 61 (59%)
> Partial checks: 7 (.06%)
> Doesn't unmap: 15 (14.5%)
> Good: 20 (19%)
>
> In summary a large % of the cases (> 50%) go unchecked. That raises the
> following questions:
>
> When do mapping errors get detected?
> How often do these errors occur?
> Why don't we see failures related to missing dma mapping error checks?
> Are they silent failures?
>
> Based on what I found, I am not too eager to remove swiotlb overflow support
> which would increase the probability of returning dma mapping errors.
>
> However I propose the following to gather more information:
>
> - Change swiotlb to log (pr_info or pr_debug) cases where overflow buffer is
> triggered. (This is a delta on the disable swiotlb patch I sent a few weeks
> ago - References in this posting).

As opposed to printk(KERN_ERR ? Why?

> - Change dma_map_single() and dma_map_page() to track how many times they
> return before attempting to fix all the places that don't do dma mapping
> error checks. (Maybe a counter that keeps track, pr_* is not an option).

Perhaps this should be done in the DMA debug API instead?

>
> Comments, thoughts on the analysis and proposal are welcome.
>
> -- Shuah


\
 
 \ /
  Last update: 2012-08-17 17:01    [W:0.063 / U:0.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site