diff mbox series

[2/5] kernel/dma: remove unnecessary unmap_kernel_range

Message ID 20210126045404.2492588-3-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series mm/vmalloc: cleanup after hugepage series | expand

Commit Message

Nicholas Piggin Jan. 26, 2021, 4:54 a.m. UTC
vunmap will remove ptes.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 kernel/dma/remap.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Christoph Hellwig Jan. 26, 2021, 6:38 a.m. UTC | #1
On Tue, Jan 26, 2021 at 02:54:01PM +1000, Nicholas Piggin wrote:
> vunmap will remove ptes.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Konrad Rzeszutek Wilk Jan. 26, 2021, 10:08 p.m. UTC | #2
On Tue, Jan 26, 2021 at 02:54:01PM +1000, Nicholas Piggin wrote:
> vunmap will remove ptes.

Should there be some ASSERT after the vunmap to make sure that is the
case? 
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  kernel/dma/remap.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
> index 905c3fa005f1..b4526668072e 100644
> --- a/kernel/dma/remap.c
> +++ b/kernel/dma/remap.c
> @@ -66,6 +66,5 @@ void dma_common_free_remap(void *cpu_addr, size_t size)
>  		return;
>  	}
>  
> -	unmap_kernel_range((unsigned long)cpu_addr, PAGE_ALIGN(size));
>  	vunmap(cpu_addr);
>  }
> -- 
> 2.23.0
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
Christoph Hellwig Jan. 27, 2021, 7:10 a.m. UTC | #3
On Tue, Jan 26, 2021 at 05:08:46PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 26, 2021 at 02:54:01PM +1000, Nicholas Piggin wrote:
> > vunmap will remove ptes.
> 
> Should there be some ASSERT after the vunmap to make sure that is the
> case? 

Not really.  removing the PTEs is the whole point of vunmap.  Everything
else is just house keeping.
Nicholas Piggin Jan. 27, 2021, 11:44 p.m. UTC | #4
Excerpts from Christoph Hellwig's message of January 27, 2021 5:10 pm:
> On Tue, Jan 26, 2021 at 05:08:46PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Tue, Jan 26, 2021 at 02:54:01PM +1000, Nicholas Piggin wrote:
>> > vunmap will remove ptes.
>> 
>> Should there be some ASSERT after the vunmap to make sure that is the
>> case? 
> 
> Not really.  removing the PTEs is the whole point of vunmap.  Everything
> else is just house keeping.

Agree. I did double check this and wrote a quick test to check ptes were 
there before the vunmap and cleared after, just to make sure I didn't 
make a silly mistake with the patch. But in general drivers should be 
able to trust code behind the API call will do the right thing. Such 
assertions should go in the vunmap() implementation as appropriate.

Thanks,
Nick
diff mbox series

Patch

diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
index 905c3fa005f1..b4526668072e 100644
--- a/kernel/dma/remap.c
+++ b/kernel/dma/remap.c
@@ -66,6 +66,5 @@  void dma_common_free_remap(void *cpu_addr, size_t size)
 		return;
 	}
 
-	unmap_kernel_range((unsigned long)cpu_addr, PAGE_ALIGN(size));
 	vunmap(cpu_addr);
 }