[v3,04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent
diff mbox series

Message ID 20190715031723.6375-1-huangfq.daxian@gmail.com
State Not Applicable, archived
Delegated to: Andy Gross
Headers show
Series
  • Untitled series #145753
Related show

Commit Message

Fuqian Huang July 15, 2019, 3:17 a.m. UTC
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 <huangfq.daxian@gmail.com>
---
Changes in v3:
  - Use actual commit rather than the merge commit in the commit message

 drivers/dma/qcom/hidma_ll.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Sinan Kaya July 15, 2019, 4:17 a.m. UTC | #1
On 7/14/2019 11:17 PM, 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 <huangfq.daxian@gmail.com>

I don't see SWIO or ARM64 IOMMU drivers getting impacted by
the mentioned change above (518a2f1925c3).

 arch/alpha/kernel/pci_iommu.c    | 2 +-
 arch/arc/mm/dma.c                | 2 +-
 arch/c6x/mm/dma-coherent.c       | 5 ++++-
 arch/m68k/kernel/dma.c           | 2 +-
 arch/microblaze/mm/consistent.c  | 2 +-
 arch/openrisc/kernel/dma.c       | 2 +-
 arch/parisc/kernel/pci-dma.c     | 4 ++--
 arch/s390/pci/pci_dma.c          | 2 +-
 arch/sparc/kernel/ioport.c       | 2 +-
 arch/sparc/mm/io-unit.c          | 2 +-
 arch/sparc/mm/iommu.c            | 2 +-
 arch/xtensa/kernel/pci-dma.c     | 2 +-
 drivers/misc/mic/host/mic_boot.c | 2 +-
 kernel/dma/virt.c                | 2 +-
 14 files changed, 18 insertions(+), 15 deletions(-)

How does this new behavior apply globally?
Fuqian Huang July 15, 2019, 5:43 a.m. UTC | #2
Sinan Kaya <Okaya@kernel.org> 於 2019年7月15日週一 下午12:17寫道:
>
> On 7/14/2019 11:17 PM, 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 <huangfq.daxian@gmail.com>
>
> I don't see SWIO or ARM64 IOMMU drivers getting impacted by
> the mentioned change above (518a2f1925c3).
>
> How does this new behavior apply globally?
>
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?
Sinan Kaya July 15, 2019, 3:17 p.m. UTC | #3
On 7/15/2019 1:43 AM, 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'd like to hear from Robin Murphy that arm smmu driver follows this as
well.
Robin Murphy July 16, 2019, 11:35 a.m. UTC | #4
On 15/07/2019 16:17, Sinan Kaya wrote:
> On 7/15/2019 1:43 AM, 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'd like to hear from Robin Murphy that arm smmu driver follows this as
> well.

I'd be lying if I said it did.

...but only because that's never been part of the SMMU driver's 
responsibility either way. The iommu-dma layer however, and thus the 
respective arm64 iommu_dma_ops, has always zeroed allocations right from 
its inception.

518a2f1925c3 was just cleaning up the last of the stragglers which 
*weren't* already clearing buffers anyway, such that we could formalise 
that behaviour into the API.

Robin.
Sinan Kaya July 16, 2019, 3:10 p.m. UTC | #5
On 7/16/2019 7:35 AM, Robin Murphy wrote:
> On 15/07/2019 16:17, Sinan Kaya wrote:
>> On 7/15/2019 1:43 AM, 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'd like to hear from Robin Murphy that arm smmu driver follows this as
>> well.
> 
> I'd be lying if I said it did.
> 
> ...but only because that's never been part of the SMMU driver's
> responsibility either way. The iommu-dma layer however, and thus the
> respective arm64 iommu_dma_ops, has always zeroed allocations right from
> its inception.
> 
> 518a2f1925c3 was just cleaning up the last of the stragglers which
> *weren't* already clearing buffers anyway, such that we could formalise
> that behaviour into the API.

Thanks for confirming the behavior for arm64 arch.

Acked-by: Sinan Kaya <okaya@kernel.org>
Vinod Koul Aug. 8, 2019, 12:22 p.m. UTC | #6
On 15-07-19, 11:17, 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.

Applied, thanks

Patch
diff mbox series

diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c
index 5bf8b145c427..bb4471e84e48 100644
--- a/drivers/dma/qcom/hidma_ll.c
+++ b/drivers/dma/qcom/hidma_ll.c
@@ -749,7 +749,6 @@  struct hidma_lldev *hidma_ll_init(struct device *dev, u32 nr_tres,
 	if (!lldev->tre_ring)
 		return NULL;
 
-	memset(lldev->tre_ring, 0, (HIDMA_TRE_SIZE + 1) * nr_tres);
 	lldev->tre_ring_size = HIDMA_TRE_SIZE * nr_tres;
 	lldev->nr_tres = nr_tres;
 
@@ -769,7 +768,6 @@  struct hidma_lldev *hidma_ll_init(struct device *dev, u32 nr_tres,
 	if (!lldev->evre_ring)
 		return NULL;
 
-	memset(lldev->evre_ring, 0, (HIDMA_EVRE_SIZE + 1) * nr_tres);
 	lldev->evre_ring_size = HIDMA_EVRE_SIZE * nr_tres;
 
 	/* the EVRE ring has to be EVRE_SIZE aligned */