2017-08-08 09:15:51

by Michael Moese

[permalink] [raw]
Subject: [PATCH] Introduce dmam_zalloc_coherent()

All memory allocation functions have a pendant for allocating zeroed
memory, but dmam_alloc_coherent does not have such a pendant.
However, it is easier to read dmam_zalloc_coherent than passing an extra
flag or, even worse, see memset() after the allocation.
This patch adds an inline function dmam_zalloc_coherent(), exactly like
the implementation of dma_zalloc_coherent().

Signed-off-by: Michael Moese <[email protected]>
---
include/linux/dma-mapping.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 03c0196a6f24..cf6cbda76ee2 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -761,6 +761,15 @@ extern void dmam_free_coherent(struct device *dev, size_t size, void *vaddr,
extern void *dmam_alloc_attrs(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp,
unsigned long attrs);
+
+static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flag)
+{
+ void *ret = dmam_alloc_coherent(dev, size, dma_handle,
+ flag | __GFP_ZERO);
+ return ret;
+}
+
#ifdef CONFIG_HAVE_GENERIC_DMA_COHERENT
extern int dmam_declare_coherent_memory(struct device *dev,
phys_addr_t phys_addr,
--
2.13.1


2017-08-09 13:37:08

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] Introduce dmam_zalloc_coherent()

On Tue, Aug 08, 2017 at 11:15:35AM +0200, Michael Moese wrote:
> All memory allocation functions have a pendant for allocating zeroed
> memory, but dmam_alloc_coherent does not have such a pendant.
> However, it is easier to read dmam_zalloc_coherent than passing an extra
> flag or, even worse, see memset() after the allocation.
> This patch adds an inline function dmam_zalloc_coherent(), exactly like
> the implementation of dma_zalloc_coherent().

I'm a bit worried about the __GFP_ZERO as we have lots of non-kmalloc
implementations of these. But on the other hand we already implement
dma_zalloc_coherent the same way, which means we'd already buggy.

So I plan to apply this for 4.14, but I also plan to spend some time
to implement all the existin alloc ops to make sure it's going to work
fine.

2017-08-09 17:45:14

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] Introduce dmam_zalloc_coherent()

Hi Michael,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc4 next-20170808]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Michael-Moese/Introduce-dmam_zalloc_coherent/20170810-011949
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All errors (new ones prefixed by >>):

In file included from include/linux/skbuff.h:34:0,
from include/net/net_namespace.h:34,
from include/linux/inet.h:46,
from include/linux/sunrpc/msg_prot.h:203,
from include/linux/sunrpc/auth.h:15,
from include/linux/nfs_fs.h:29,
from init/do_mounts.c:32:
>> include/linux/dma-mapping.h:765:21: error: redefinition of 'dma_zalloc_coherent'
static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
^~~~~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:691:21: note: previous definition of 'dma_zalloc_coherent' was here
static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
^~~~~~~~~~~~~~~~~~~

vim +/dma_zalloc_coherent +765 include/linux/dma-mapping.h

753
754 /*
755 * Managed DMA API
756 */
757 extern void *dmam_alloc_coherent(struct device *dev, size_t size,
758 dma_addr_t *dma_handle, gfp_t gfp);
759 extern void dmam_free_coherent(struct device *dev, size_t size, void *vaddr,
760 dma_addr_t dma_handle);
761 extern void *dmam_alloc_attrs(struct device *dev, size_t size,
762 dma_addr_t *dma_handle, gfp_t gfp,
763 unsigned long attrs);
764
> 765 static inline void *dma_zalloc_coherent(struct device *dev, size_t size,
766 dma_addr_t *dma_handle, gfp_t flag)
767 {
768 void *ret = dmam_alloc_coherent(dev, size, dma_handle,
769 flag | __GFP_ZERO);
770 return ret;
771 }
772

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.25 kB)
.config.gz (6.51 kB)
Download all attachments

2017-08-10 06:59:00

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] Introduce dmam_zalloc_coherent()

Hi Christoph,

On 2017-08-09 15:37, Christoph Hellwig wrote:
> On Tue, Aug 08, 2017 at 11:15:35AM +0200, Michael Moese wrote:
>> All memory allocation functions have a pendant for allocating zeroed
>> memory, but dmam_alloc_coherent does not have such a pendant.
>> However, it is easier to read dmam_zalloc_coherent than passing an extra
>> flag or, even worse, see memset() after the allocation.
>> This patch adds an inline function dmam_zalloc_coherent(), exactly like
>> the implementation of dma_zalloc_coherent().
> I'm a bit worried about the __GFP_ZERO as we have lots of non-kmalloc
> implementations of these. But on the other hand we already implement
> dma_zalloc_coherent the same way, which means we'd already buggy.
>
> So I plan to apply this for 4.14, but I also plan to spend some time
> to implement all the existin alloc ops to make sure it's going to work
> fine.

Frankly, since introducing dma_mmap_coherent, dma_alloc_coherent already
clears allocated buffers to avoid potential information leak to userspace.
There are even drivers that rely on such behavior, see
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-April/338804.html

Maybe it would make sense to properly document it and then convert
dma_zalloc* to standard dma_alloc_* calls?

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland