Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-24 Thread Joerg Roedel
On Thu, Oct 18, 2012 at 02:00:58PM -0600, Shuah Khan wrote:
> Enhance the document to discuss the importance of dma mapping error checks
> after dma_map_single() and dma_map_page() calls. Also added usage examples
> that include unmap examples in error paths when dma mapping error is returned.
> Includes correct and incorrect usages to high light some common mistakes in
> error paths especially when dma mapping fails when more than one dma mapping
> call is made.
> 
> Signed-off-by: Shuah Khan 

Applied, thanks.

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-24 Thread Joerg Roedel
On Thu, Oct 18, 2012 at 02:00:58PM -0600, Shuah Khan wrote:
 Enhance the document to discuss the importance of dma mapping error checks
 after dma_map_single() and dma_map_page() calls. Also added usage examples
 that include unmap examples in error paths when dma mapping error is returned.
 Includes correct and incorrect usages to high light some common mistakes in
 error paths especially when dma mapping fails when more than one dma mapping
 call is made.
 
 Signed-off-by: Shuah Khan shuah.k...@hp.com

Applied, thanks.

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-19 Thread Konrad Rzeszutek Wilk
On Thu, Oct 18, 2012 at 02:00:58PM -0600, Shuah Khan wrote:
> Enhance the document to discuss the importance of dma mapping error checks
> after dma_map_single() and dma_map_page() calls. Also added usage examples
> that include unmap examples in error paths when dma mapping error is returned.
> Includes correct and incorrect usages to high light some common mistakes in
> error paths especially when dma mapping fails when more than one dma mapping
> call is made.
> 
> Signed-off-by: Shuah Khan 

Reviewed-by: Konrad Rzeszutek Wilk 
> ---
>  Documentation/DMA-API-HOWTO.txt |  126 
> +++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
> index a0b6250..4a4fb29 100644
> --- a/Documentation/DMA-API-HOWTO.txt
> +++ b/Documentation/DMA-API-HOWTO.txt
> @@ -468,11 +468,46 @@ To map a single region, you do:
>   size_t size = buffer->len;
>  
>   dma_handle = dma_map_single(dev, addr, size, direction);
> + if (dma_mapping_error(dma_handle)) {
> + /*
> +  * reduce current DMA mapping usage,
> +  * delay and try again later or
> +  * reset driver.
> +  */
> + goto map_error_handling;
> + }
>  
>  and to unmap it:
>  
>   dma_unmap_single(dev, dma_handle, size, direction);
>  
> +You should call dma_mapping_error() as dma_map_single() could fail and return
> +error. Not all dma implementations support dma_mapping_error() interface.
> +However, it is a good practice to call dma_mapping_error() interface, which
> +will invoke the generic mapping error check interface. Doing so will ensure
> +that the mapping code will work correctly on all dma implementations without
> +any dependency on the specifics of the underlying implementation. Using the
> +returned address without checking for errors could result in failures ranging
> +from panics to silent data corruption. Couple of example of incorrect ways to
> +check for errors that make assumptions about the underlying dma 
> implementation
> +are as follows and these are applicable to dma_map_page() as well.
> +
> +Incorrect example 1:
> + dma_addr_t dma_handle;
> +
> + dma_handle = dma_map_single(dev, addr, size, direction);
> + if ((dma_handle & 0x != 0) || (dma_handle >= 0x100)) {
> + goto map_error;
> + }
> +
> +Incorrect example 2:
> + dma_addr_t dma_handle;
> +
> + dma_handle = dma_map_single(dev, addr, size, direction);
> + if (dma_handle == DMA_ERROR_CODE) {
> + goto map_error;
> + }
> +
>  You should call dma_unmap_single when the DMA activity is finished, e.g.
>  from the interrupt which told you that the DMA transfer is done.
>  
> @@ -489,6 +524,14 @@ Specifically:
>   size_t size = buffer->len;
>  
>   dma_handle = dma_map_page(dev, page, offset, size, direction);
> + if (dma_mapping_error(dma_handle)) {
> + /*
> +  * reduce current DMA mapping usage,
> +  * delay and try again later or
> +  * reset driver.
> +  */
> + goto map_error_handling;
> + }
>  
>   ...
>  
> @@ -496,6 +539,12 @@ Specifically:
>  
>  Here, "offset" means byte offset within the given page.
>  
> +You should call dma_mapping_error() as dma_map_page() could fail and return
> +error as outlined under the dma_map_single() discussion.
> +
> +You should call dma_unmap_page when the DMA activity is finished, e.g.
> +from the interrupt which told you that the DMA transfer is done.
> +
>  With scatterlists, you map a region gathered from several regions by:
>  
>   int i, count = dma_map_sg(dev, sglist, nents, direction);
> @@ -578,6 +627,14 @@ to use the dma_sync_*() interfaces.
>   dma_addr_t mapping;
>  
>   mapping = dma_map_single(cp->dev, buffer, len, DMA_FROM_DEVICE);
> + if (dma_mapping_error(dma_handle)) {
> + /*
> +  * reduce current DMA mapping usage,
> +  * delay and try again later or
> +  * reset driver.
> +  */
> + goto map_error_handling;
> + }
>  
>   cp->rx_buf = buffer;
>   cp->rx_len = len;
> @@ -658,6 +715,75 @@ failure can be determined by:
>* delay and try again later or
>* reset driver.
>*/
> + goto map_error_handling;
> + }
> +
> +- unmap pages that are already mapped, when mapping error occurs in the 
> middle
> +  of a multiple page mapping attempt. These example are applicable to
> +  dma_map_page() as well.
> +
> +Example 1:
> + dma_addr_t dma_handle1;
> + dma_addr_t dma_handle2;
> +
> + dma_handle1 = dma_map_single(dev, addr, size, direction);
> + if (dma_mapping_error(dev, dma_handle1)) {
> + /*
> +  * reduce current DMA 

Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-19 Thread Konrad Rzeszutek Wilk
On Thu, Oct 18, 2012 at 02:00:58PM -0600, Shuah Khan wrote:
 Enhance the document to discuss the importance of dma mapping error checks
 after dma_map_single() and dma_map_page() calls. Also added usage examples
 that include unmap examples in error paths when dma mapping error is returned.
 Includes correct and incorrect usages to high light some common mistakes in
 error paths especially when dma mapping fails when more than one dma mapping
 call is made.
 
 Signed-off-by: Shuah Khan shuah.k...@hp.com

Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
 ---
  Documentation/DMA-API-HOWTO.txt |  126 
 +++
  1 file changed, 126 insertions(+)
 
 diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
 index a0b6250..4a4fb29 100644
 --- a/Documentation/DMA-API-HOWTO.txt
 +++ b/Documentation/DMA-API-HOWTO.txt
 @@ -468,11 +468,46 @@ To map a single region, you do:
   size_t size = buffer-len;
  
   dma_handle = dma_map_single(dev, addr, size, direction);
 + if (dma_mapping_error(dma_handle)) {
 + /*
 +  * reduce current DMA mapping usage,
 +  * delay and try again later or
 +  * reset driver.
 +  */
 + goto map_error_handling;
 + }
  
  and to unmap it:
  
   dma_unmap_single(dev, dma_handle, size, direction);
  
 +You should call dma_mapping_error() as dma_map_single() could fail and return
 +error. Not all dma implementations support dma_mapping_error() interface.
 +However, it is a good practice to call dma_mapping_error() interface, which
 +will invoke the generic mapping error check interface. Doing so will ensure
 +that the mapping code will work correctly on all dma implementations without
 +any dependency on the specifics of the underlying implementation. Using the
 +returned address without checking for errors could result in failures ranging
 +from panics to silent data corruption. Couple of example of incorrect ways to
 +check for errors that make assumptions about the underlying dma 
 implementation
 +are as follows and these are applicable to dma_map_page() as well.
 +
 +Incorrect example 1:
 + dma_addr_t dma_handle;
 +
 + dma_handle = dma_map_single(dev, addr, size, direction);
 + if ((dma_handle  0x != 0) || (dma_handle = 0x100)) {
 + goto map_error;
 + }
 +
 +Incorrect example 2:
 + dma_addr_t dma_handle;
 +
 + dma_handle = dma_map_single(dev, addr, size, direction);
 + if (dma_handle == DMA_ERROR_CODE) {
 + goto map_error;
 + }
 +
  You should call dma_unmap_single when the DMA activity is finished, e.g.
  from the interrupt which told you that the DMA transfer is done.
  
 @@ -489,6 +524,14 @@ Specifically:
   size_t size = buffer-len;
  
   dma_handle = dma_map_page(dev, page, offset, size, direction);
 + if (dma_mapping_error(dma_handle)) {
 + /*
 +  * reduce current DMA mapping usage,
 +  * delay and try again later or
 +  * reset driver.
 +  */
 + goto map_error_handling;
 + }
  
   ...
  
 @@ -496,6 +539,12 @@ Specifically:
  
  Here, offset means byte offset within the given page.
  
 +You should call dma_mapping_error() as dma_map_page() could fail and return
 +error as outlined under the dma_map_single() discussion.
 +
 +You should call dma_unmap_page when the DMA activity is finished, e.g.
 +from the interrupt which told you that the DMA transfer is done.
 +
  With scatterlists, you map a region gathered from several regions by:
  
   int i, count = dma_map_sg(dev, sglist, nents, direction);
 @@ -578,6 +627,14 @@ to use the dma_sync_*() interfaces.
   dma_addr_t mapping;
  
   mapping = dma_map_single(cp-dev, buffer, len, DMA_FROM_DEVICE);
 + if (dma_mapping_error(dma_handle)) {
 + /*
 +  * reduce current DMA mapping usage,
 +  * delay and try again later or
 +  * reset driver.
 +  */
 + goto map_error_handling;
 + }
  
   cp-rx_buf = buffer;
   cp-rx_len = len;
 @@ -658,6 +715,75 @@ failure can be determined by:
* delay and try again later or
* reset driver.
*/
 + goto map_error_handling;
 + }
 +
 +- unmap pages that are already mapped, when mapping error occurs in the 
 middle
 +  of a multiple page mapping attempt. These example are applicable to
 +  dma_map_page() as well.
 +
 +Example 1:
 + dma_addr_t dma_handle1;
 + dma_addr_t dma_handle2;
 +
 + dma_handle1 = dma_map_single(dev, addr, size, direction);
 + if (dma_mapping_error(dev, dma_handle1)) {
 + /*
 +  * reduce current DMA mapping usage,
 +  * delay and try again later or
 +  * reset driver.
 +

Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-18 Thread Jesper Juhl
On Thu, 18 Oct 2012, Shuah Khan wrote:

> Enhance the document to discuss the importance of dma mapping error checks
> after dma_map_single() and dma_map_page() calls. Also added usage examples
> that include unmap examples in error paths when dma mapping error is returned.
> Includes correct and incorrect usages to high light some common mistakes in
> error paths especially when dma mapping fails when more than one dma mapping
> call is made.
> 
> Signed-off-by: Shuah Khan 
[...]

Nit picking :

> +Example 2: (if buffers are allocated a loop, unmap all mapped buffers when
> + mapping error is detected in the middle)
> +

"... if buffers are allocated in a loop..."

-- 
Jesper Juhlhttp://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] Documentation DMA-API-HOWTO.txt Add dma mapping error check usage examples

2012-10-18 Thread Jesper Juhl
On Thu, 18 Oct 2012, Shuah Khan wrote:

 Enhance the document to discuss the importance of dma mapping error checks
 after dma_map_single() and dma_map_page() calls. Also added usage examples
 that include unmap examples in error paths when dma mapping error is returned.
 Includes correct and incorrect usages to high light some common mistakes in
 error paths especially when dma mapping fails when more than one dma mapping
 call is made.
 
 Signed-off-by: Shuah Khan shuah.k...@hp.com
[...]

Nit picking :

 +Example 2: (if buffers are allocated a loop, unmap all mapped buffers when
 + mapping error is detected in the middle)
 +

... if buffers are allocated in a loop...

-- 
Jesper Juhl j...@chaosbits.net   http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/