[PATCH] Introduce dmam_zalloc_coherent()

[PATCH] Introduce dmam_zalloc_coherent()

[Michael Moese]

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 <michael.moese@men.de>
---
 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

Re: [PATCH] Introduce dmam_zalloc_coherent()

[Christoph Hellwig]

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.

Re: [PATCH] Introduce dmam_zalloc_coherent()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2301 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6665 bytes --]

Re: [PATCH] Introduce dmam_zalloc_coherent()

[Marek Szyprowski]

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