dma mapping error check analysis

* dma mapping error check analysis
@ 2012-08-10 22:46 Shuah Khan
  2012-08-17 14:11 ` Konrad Rzeszutek Wilk
  2012-08-24 21:14 ` [PATCH] swiotlb: Add support to disable swiotlb overflow buffer with deprecation warning Shuah Khan
  0 siblings, 2 replies; 6+ messages in thread
From: Shuah Khan @ 2012-08-10 22:46 UTC (permalink / raw)
  To: fujita.tomonori, akpm, paul.gortmaker, bhelgaas, amwang,
	Konrad Rzeszutek Wilk
  Cc: LKML, shuahkhan

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).
- 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).

Comments, thoughts on the analysis and proposal are welcome.

-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread