2021-01-26 23:30:08

by Nicholas Piggin

[permalink] [raw]
Subject: [PATCH 2/5] kernel/dma: remove unnecessary unmap_kernel_range

vunmap will remove ptes.

Cc: Christoph Hellwig <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: [email protected]
Signed-off-by: Nicholas Piggin <[email protected]>
---
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


2021-01-27 19:58:25

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 2/5] kernel/dma: remove unnecessary unmap_kernel_range

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 <[email protected]>
> Cc: Marek Szyprowski <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: [email protected]
> Signed-off-by: Nicholas Piggin <[email protected]>
> ---
> 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
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

2021-01-27 21:32:09

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/5] kernel/dma: remove unnecessary unmap_kernel_range

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.

2021-01-28 00:49:07

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH 2/5] kernel/dma: remove unnecessary unmap_kernel_range

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