About handling dma_mapping_error result after dma_map_*

About handling dma_mapping_error result after dma_map_*

[Haneen Mohammed]

Hello,

I have a question about handling dma_mapping_error result. Is it enough
to add a warning after it? Because I have found two missing
dma_mapping_error in the drm/via driver but I wasn't sure how to handle 
the return value if an error was found. Below is the code mentioned:

---
 drivers/gpu/drm/via/via_dmablit.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
index 98aae98..5c11756 100644
--- a/drivers/gpu/drm/via/via_dmablit.c
+++ b/drivers/gpu/drm/via/via_dmablit.c
@@ -132,12 +132,19 @@ via_map_blit_for_device(struct pci_dev *pdev,
 								VIA_PFN(first_addr)],
 						     VIA_PGOFF(cur_mem), remaining_len,
 						     vsg->direction);
+				if (dma_mapping_error(&pdev->dev,
+						      desc_ptr->mem_addr))
+					dev_warn(&pdev->dev, "DMA mapping failed\n");
+
 				desc_ptr->dev_addr = cur_fb;
 
 				desc_ptr->size = remaining_len;
 				desc_ptr->next = (uint32_t) next;
 				next = dma_map_single(&pdev->dev, desc_ptr, sizeof(*desc_ptr),
 						      DMA_TO_DEVICE);
+				if (dma_mapping_error(&pdev->dev,
+						      next))
+					dev_warn(&pdev->dev, "DMA mapping failed\n");
 				desc_ptr++;
 				if (++num_descriptors_this_page >= vsg->descriptors_per_page) {
 					num_descriptors_this_page = 0;
-- 
2.7.4

Thank you!
Haneen

Re: About handling dma_mapping_error result after dma_map_*

[Julia Lawall]

On Fri, 15 Sep 2017, Haneen Mohammed wrote:

> Hello,
>
> I have a question about handling dma_mapping_error result. Is it enough
> to add a warning after it? Because I have found two missing
> dma_mapping_error in the drm/via driver but I wasn't sure how to handle
> the return value if an error was found. Below is the code mentioned:

Intuitively, I think you need to get out of the function and report
failure to the call site.  It's like kmalloc.  If you don't get the memory
you need, there is not much you can do afterwards.  But see what the drm
people say before embarking on what would be a somewhat complex change.
Perhaps there is some reason why it cannot fail.

julia

>
> ---
>  drivers/gpu/drm/via/via_dmablit.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
> index 98aae98..5c11756 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -132,12 +132,19 @@ via_map_blit_for_device(struct pci_dev *pdev,
>  								VIA_PFN(first_addr)],
>  						     VIA_PGOFF(cur_mem), remaining_len,
>  						     vsg->direction);
> +				if (dma_mapping_error(&pdev->dev,
> +						      desc_ptr->mem_addr))
> +					dev_warn(&pdev->dev, "DMA mapping failed\n");
> +
>  				desc_ptr->dev_addr = cur_fb;
>
>  				desc_ptr->size = remaining_len;
>  				desc_ptr->next = (uint32_t) next;
>  				next = dma_map_single(&pdev->dev, desc_ptr, sizeof(*desc_ptr),
>  						      DMA_TO_DEVICE);
> +				if (dma_mapping_error(&pdev->dev,
> +						      next))
> +					dev_warn(&pdev->dev, "DMA mapping failed\n");
>  				desc_ptr++;
>  				if (++num_descriptors_this_page >= vsg->descriptors_per_page) {
>  					num_descriptors_this_page = 0;
> --
> 2.7.4
>
> Thank you!
> Haneen
>