2019-07-15 03:20:52

by Fuqian Huang

[permalink] [raw]
Subject: [PATCH v3 15/24] media: exynos4-is: Remove call to memset after dma_alloc_coherent

In commit 518a2f1925c3
("dma-mapping: zero memory returned from dma_alloc_*"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed

Signed-off-by: Fuqian Huang <[email protected]>
---
Changes in v3:
- Use actual commit rather than the merge commit in the commit message

drivers/media/platform/exynos4-is/fimc-is.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index e043d55133a3..77633e356305 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -341,7 +341,6 @@ static int fimc_is_alloc_cpu_memory(struct fimc_is *is)
return -ENOMEM;

is->memory.size = FIMC_IS_CPU_MEM_SIZE;
- memset(is->memory.vaddr, 0, is->memory.size);

dev_info(dev, "FIMC-IS CPU memory base: %#x\n", (u32)is->memory.paddr);

--
2.11.0


2019-07-15 09:01:39

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH v3 15/24] media: exynos4-is: Remove call to memset after dma_alloc_coherent

On 7/15/19 05:18, Fuqian Huang wrote:
> In commit 518a2f1925c3
> ("dma-mapping: zero memory returned from dma_alloc_*"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed

I can't see any changes in the above mentioned commit that would
make dma_alloc_coherent() (arm_coherent_dma_alloc()) zeroing its
returned memory. Maybe you need to add a reference to some other
instead?

> Signed-off-by: Fuqian Huang <[email protected]>

> drivers/media/platform/exynos4-is/fimc-is.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
> index e043d55133a3..77633e356305 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is.c
> @@ -341,7 +341,6 @@ static int fimc_is_alloc_cpu_memory(struct fimc_is *is)
> return -ENOMEM;
>
> is->memory.size = FIMC_IS_CPU_MEM_SIZE;
> - memset(is->memory.vaddr, 0, is->memory.size);
--
Thanks,
Sylwester

2019-07-15 09:45:00

by Fuqian Huang

[permalink] [raw]
Subject: Re: [PATCH v3 15/24] media: exynos4-is: Remove call to memset after dma_alloc_coherent

Sylwester Nawrocki <[email protected]> 於 2019年7月15日週一 下午5:00寫道:
>
> On 7/15/19 05:18, Fuqian Huang wrote:
> > In commit 518a2f1925c3
> > ("dma-mapping: zero memory returned from dma_alloc_*"),
> > dma_alloc_coherent has already zeroed the memory.
> > So memset is not needed
>
> I can't see any changes in the above mentioned commit that would
> make dma_alloc_coherent() (arm_coherent_dma_alloc()) zeroing its
> returned memory. Maybe you need to add a reference to some other
> instead?
>
In the last version patch set, I referenced the commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of
git://git.infradead.org/users/hch/dma-mapping")
in the commit log.
The merged commit mentions that
"ensure dma_alloc_coherent returns zeroed memory to
avoid kernel data leaks through userspace.
We already did this for most common architectures,
but this ensures we do it everywhere."
dma_alloc_coherent has already zeroed the memory during allocation
and the commit also deprecates dma_zalloc_coherent.
Greg and other maintainer told me to use the actual commit
rather than the merged commit.
So I reference the commit that ensures the dma_alloc_coherent to
returns zeroed memory every where.
Maybe this belongs to the `most common achitectures` and is not impacted
by the mentioned change.
Should I rewrite the commit log? Just mention that dma_alloc_coherent
has already zeroed the memory and not to reference the commit?

2019-07-15 11:45:38

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH v3 15/24] media: exynos4-is: Remove call to memset after dma_alloc_coherent

On 7/15/19 11:43, Fuqian Huang wrote:
> Should I rewrite the commit log? Just mention that dma_alloc_coherent
> has already zeroed the memory and not to reference the commit?

I don't think it is really needed, since at hash 518a2f1925c3 dma_alloc_coherent()
already zeroes the memory, so in fact all statements in your current commit
message are true.

--
Regards,
Sylwester

2019-07-15 11:47:30

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH v3 15/24] media: exynos4-is: Remove call to memset after dma_alloc_coherent

On 7/15/19 05:18, Fuqian Huang wrote:
> In commit 518a2f1925c3
> ("dma-mapping: zero memory returned from dma_alloc_*"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed
>
> Signed-off-by: Fuqian Huang <[email protected]>

Reviewed-by: Sylwester Nawrocki <[email protected]>