diff mbox

[4/4] ARM: dma-mapping: Skip cache maint for invalid page

Message ID 1346154232-13299-4-git-send-email-hdoyu@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hiroshi DOYU Aug. 28, 2012, 11:43 a.m. UTC
Skip cache maint for invalid page but warn it.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mm/dma-mapping.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Comments

Marek Szyprowski Aug. 28, 2012, 9:58 p.m. UTC | #1
Hello,

On 8/28/2012 1:43 PM, Hiroshi Doyu wrote:

> Skip cache maint for invalid page but warn it.
>
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>   arch/arm/mm/dma-mapping.c |    9 +++++++++
>   1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 2621282..0368702 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1589,6 +1589,9 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
>   	if (!iova)
>   		return;
>
> +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> +		return;
> +
>   	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
>   		__dma_page_dev_to_cpu(page, offset, size, dir);
>
> @@ -1607,6 +1610,9 @@ static void arm_iommu_sync_single_for_cpu(struct device *dev,
>   	if (!iova)
>   		return;
>
> +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> +		return;
> +
>   	__dma_page_dev_to_cpu(page, offset, size, dir);
>   }
>
> @@ -1621,6 +1627,9 @@ static void arm_iommu_sync_single_for_device(struct device *dev,
>   	if (!iova)
>   		return;
>
> +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> +		return;
> +
>   	__dma_page_cpu_to_dev(page, offset, size, dir);
>   }

I really wonder what has happened in your system that you require
such paranoid checks. All of the above functions are called on the
scatter-list which has been first passed to dma_map_sg(). IMHO it
makes much more sense to add a pfn_valid() check in dma_map_sg()
and return error if the provided scatter list is not valid instead
of these WARN_ONs.

Best regards
Hiroshi DOYU Aug. 29, 2012, 4:49 a.m. UTC | #2
Hi Marek,

Marek Szyprowski <m.szyprowski@samsung.com> wrote @ Tue, 28 Aug 2012 23:58:48 +0200:

> Hello,
> 
> On 8/28/2012 1:43 PM, Hiroshi Doyu wrote:
> 
> > Skip cache maint for invalid page but warn it.
> >
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >   arch/arm/mm/dma-mapping.c |    9 +++++++++
> >   1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 2621282..0368702 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -1589,6 +1589,9 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
> >   	if (!iova)
> >   		return;
> >
> > +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> > +		return;
> > +
> >   	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
> >   		__dma_page_dev_to_cpu(page, offset, size, dir);
> >
> > @@ -1607,6 +1610,9 @@ static void arm_iommu_sync_single_for_cpu(struct device *dev,
> >   	if (!iova)
> >   		return;
> >
> > +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> > +		return;
> > +
> >   	__dma_page_dev_to_cpu(page, offset, size, dir);
> >   }
> >
> > @@ -1621,6 +1627,9 @@ static void arm_iommu_sync_single_for_device(struct device *dev,
> >   	if (!iova)
> >   		return;
> >
> > +	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
> > +		return;
> > +
> >   	__dma_page_cpu_to_dev(page, offset, size, dir);
> >   }
> 
> I really wonder what has happened in your system that you require
> such paranoid checks. All of the above functions are called on the
> scatter-list which has been first passed to dma_map_sg(). IMHO it
> makes much more sense to add a pfn_valid() check in dma_map_sg()
> and return error if the provided scatter list is not valid instead
> of these WARN_ONs.

Actually one device driver hit Oops at unmap_page() since this
function could be called directly from device drivers, and
WARN_ON(dump_stack) helped to find out which device driver did
something wrong. IIRC, the following functions can be called directly
from device drivers?

static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
				  size_t size, enum dma_data_direction dir);
static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
					   size_t size,
					   enum dma_data_direction dir);
static inline void dma_sync_single_for_device(struct device *dev,
					      dma_addr_t addr, size_t size,
					      enum dma_data_direction dir);
diff mbox

Patch

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 2621282..0368702 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1589,6 +1589,9 @@  static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 	if (!iova)
 		return;
 
+	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
+		return;
+
 	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
 		__dma_page_dev_to_cpu(page, offset, size, dir);
 
@@ -1607,6 +1610,9 @@  static void arm_iommu_sync_single_for_cpu(struct device *dev,
 	if (!iova)
 		return;
 
+	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
+		return;
+
 	__dma_page_dev_to_cpu(page, offset, size, dir);
 }
 
@@ -1621,6 +1627,9 @@  static void arm_iommu_sync_single_for_device(struct device *dev,
 	if (!iova)
 		return;
 
+	if (WARN_ON(!pfn_valid(page_to_pfn(page))))
+		return;
+
 	__dma_page_cpu_to_dev(page, offset, size, dir);
 }