Currently a call to dma_pool_alloc() with a ___GFP_ZERO flag returns
a non-zeroed memory region.
This patchset adds support for the __GFP_ZERO flag to dma_pool_alloc(),
adds 2 wrapper functions for allocing zeroed memory from a pool,
and provides a coccinelle script for finding & replacing instances of
dma_pool_alloc() followed by memset(0) with a single dma_pool_zalloc() call.
Changes from v1 to v2:
- don't memset() POOL_POISON_ALLOCATED in dma_pool_alloc() if
__GFP_ZERO is set
- Ran test to see how often pool_alloc_page() is called
There was some concern that this always calls memset() to zero,
instead of passing __GFP_ZERO into the page allocator.
[https://lkml.org/lkml/2015/7/15/881]
I ran a test on my system to get an idea of how often dma_pool_alloc()
calls into pool_alloc_page().
After Boot: [ 30.119863] alloc_calls:541, page_allocs:7
After an hour: [ 3600.951031] alloc_calls:9566, page_allocs:12
After copying 1GB file onto a USB drive:
[ 4260.657148] alloc_calls:17225, page_allocs:12
It doesn't look like dma_pool_alloc() calls down to the page allocator
very often (at least on my system).
Sean O. Stalley (4):
mm: Add support for __GFP_ZERO flag to dma_pool_alloc()
mm: Add dma_pool_zalloc() call to DMA API
pci: mm: Add pci_pool_zalloc() call
coccinelle: mm: scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
Documentation/DMA-API.txt | 7 ++
include/linux/dmapool.h | 6 ++
include/linux/pci.h | 2 +
mm/dmapool.c | 9 ++-
.../coccinelle/api/alloc/pool_zalloc-simple.cocci | 84 ++++++++++++++++++++++
5 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
--
1.9.1
Currently the __GFP_ZERO flag is ignored by dma_pool_alloc().
Make dma_pool_alloc() zero the memory if this flag is set.
Signed-off-by: Sean O. Stalley <[email protected]>
---
mm/dmapool.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/mm/dmapool.c b/mm/dmapool.c
index fd5fe43..bd49386 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -334,7 +334,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */
spin_unlock_irqrestore(&pool->lock, flags);
- page = pool_alloc_page(pool, mem_flags);
+ page = pool_alloc_page(pool, mem_flags & (~__GFP_ZERO));
if (!page)
return NULL;
@@ -372,9 +372,14 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
break;
}
}
- memset(retval, POOL_POISON_ALLOCATED, pool->size);
+ if (!(mem_flags & __GFP_ZERO))
+ memset(retval, POOL_POISON_ALLOCATED, pool->size);
#endif
spin_unlock_irqrestore(&pool->lock, flags);
+
+ if (mem_flags & __GFP_ZERO)
+ memset(retval, 0, pool->size);
+
return retval;
}
EXPORT_SYMBOL(dma_pool_alloc);
--
1.9.1
Add a wrapper function for dma_pool_alloc() to get zeroed memory.
Signed-off-by: Sean O. Stalley <[email protected]>
---
Documentation/DMA-API.txt | 7 +++++++
include/linux/dmapool.h | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 5208840..988f757 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -104,6 +104,13 @@ crossing restrictions, pass 0 for alloc; passing 4096 says memory allocated
from this pool must not cross 4KByte boundaries.
+ void *dma_pool_zalloc(struct dma_pool *pool, gfp_t mem_flags,
+ dma_addr_t *handle)
+
+Wraps dma_pool_alloc() and also zeroes the returned memory if the
+allocation attempt succeeded.
+
+
void *dma_pool_alloc(struct dma_pool *pool, gfp_t gfp_flags,
dma_addr_t *dma_handle);
diff --git a/include/linux/dmapool.h b/include/linux/dmapool.h
index 022e34f..6d8079b 100644
--- a/include/linux/dmapool.h
+++ b/include/linux/dmapool.h
@@ -22,6 +22,12 @@ void dma_pool_destroy(struct dma_pool *pool);
void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
dma_addr_t *handle);
+static inline void *dma_pool_zalloc(struct dma_pool *pool, gfp_t mem_flags,
+ dma_addr_t *handle)
+{
+ return dma_pool_alloc(pool, mem_flags | __GFP_ZERO, handle);
+}
+
void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t addr);
/*
--
1.9.1
Add a wrapper function for pci_pool_alloc() to get zeroed memory.
Signed-off-by: Sean O. Stalley <[email protected]>
---
include/linux/pci.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 755a2cd..e6ec7d9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1176,6 +1176,8 @@ int pci_set_vga_state(struct pci_dev *pdev, bool decode,
dma_pool_create(name, &pdev->dev, size, align, allocation)
#define pci_pool_destroy(pool) dma_pool_destroy(pool)
#define pci_pool_alloc(pool, flags, handle) dma_pool_alloc(pool, flags, handle)
+#define pci_pool_zalloc(pool, flags, handle) \
+ dma_pool_zalloc(pool, flags, handle)
#define pci_pool_free(pool, vaddr, addr) dma_pool_free(pool, vaddr, addr)
enum pci_dma_burst_strategy {
--
1.9.1
add [pci|dma]_pool_zalloc coccinelle check.
replaces instances of [pci|dma]_pool_alloc() followed by memset(0)
with [pci|dma]_pool_zalloc().
Signed-off-by: Sean O. Stalley <[email protected]>
---
.../coccinelle/api/alloc/pool_zalloc-simple.cocci | 84 ++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
diff --git a/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci b/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
new file mode 100644
index 0000000..9b7eb32
--- /dev/null
+++ b/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
@@ -0,0 +1,84 @@
+///
+/// Use *_pool_zalloc rather than *_pool_alloc followed by memset with 0
+///
+// Copyright: (C) 2015 Intel Corp. GPLv2.
+// Options: --no-includes --include-headers
+//
+// Keywords: dma_pool_zalloc, pci_pool_zalloc
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+// For context mode
+//----------------------------------------------------------
+
+@depends on context@
+expression x;
+statement S;
+@@
+
+* x = \(dma_pool_alloc\|pci_pool_alloc\)(...);
+ if ((x==NULL) || ...) S
+* memset(x,0, ...);
+
+//----------------------------------------------------------
+// For patch mode
+//----------------------------------------------------------
+
+@depends on patch@
+expression x;
+expression a,b,c;
+statement S;
+@@
+
+- x = dma_pool_alloc(a,b,c);
++ x = dma_pool_zalloc(a,b,c);
+ if ((x==NULL) || ...) S
+- memset(x,0,...);
+
+@depends on patch@
+expression x;
+expression a,b,c;
+statement S;
+@@
+
+- x = pci_pool_alloc(a,b,c);
++ x = pci_pool_zalloc(a,b,c);
+ if ((x==NULL) || ...) S
+- memset(x,0,...);
+
+//----------------------------------------------------------
+// For org and report mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+expression x;
+expression a,b,c;
+statement S;
+position p;
+@@
+
+ x = @p\(dma_pool_alloc\|pci_pool_alloc\)(a,b,c);
+ if ((x==NULL) || ...) S
+ memset(x,0, ...);
+
+@script:python depends on org@
+p << r.p;
+x << r.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r.p;
+x << r.x;
+@@
+
+msg="WARNING: *_pool_zalloc should be used for %s, instead of *_pool_alloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
--
1.9.1
Acked-by: Julia Lawall <[email protected]>
On Fri, 31 Jul 2015, Sean O. Stalley wrote:
> add [pci|dma]_pool_zalloc coccinelle check.
> replaces instances of [pci|dma]_pool_alloc() followed by memset(0)
> with [pci|dma]_pool_zalloc().
>
> Signed-off-by: Sean O. Stalley <[email protected]>
> ---
> .../coccinelle/api/alloc/pool_zalloc-simple.cocci | 84 ++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> create mode 100644 scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
>
> diff --git a/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci b/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
> new file mode 100644
> index 0000000..9b7eb32
> --- /dev/null
> +++ b/scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci
> @@ -0,0 +1,84 @@
> +///
> +/// Use *_pool_zalloc rather than *_pool_alloc followed by memset with 0
> +///
> +// Copyright: (C) 2015 Intel Corp. GPLv2.
> +// Options: --no-includes --include-headers
> +//
> +// Keywords: dma_pool_zalloc, pci_pool_zalloc
> +//
> +
> +virtual context
> +virtual patch
> +virtual org
> +virtual report
> +
> +//----------------------------------------------------------
> +// For context mode
> +//----------------------------------------------------------
> +
> +@depends on context@
> +expression x;
> +statement S;
> +@@
> +
> +* x = \(dma_pool_alloc\|pci_pool_alloc\)(...);
> + if ((x==NULL) || ...) S
> +* memset(x,0, ...);
> +
> +//----------------------------------------------------------
> +// For patch mode
> +//----------------------------------------------------------
> +
> +@depends on patch@
> +expression x;
> +expression a,b,c;
> +statement S;
> +@@
> +
> +- x = dma_pool_alloc(a,b,c);
> ++ x = dma_pool_zalloc(a,b,c);
> + if ((x==NULL) || ...) S
> +- memset(x,0,...);
> +
> +@depends on patch@
> +expression x;
> +expression a,b,c;
> +statement S;
> +@@
> +
> +- x = pci_pool_alloc(a,b,c);
> ++ x = pci_pool_zalloc(a,b,c);
> + if ((x==NULL) || ...) S
> +- memset(x,0,...);
> +
> +//----------------------------------------------------------
> +// For org and report mode
> +//----------------------------------------------------------
> +
> +@r depends on org || report@
> +expression x;
> +expression a,b,c;
> +statement S;
> +position p;
> +@@
> +
> + x = @p\(dma_pool_alloc\|pci_pool_alloc\)(a,b,c);
> + if ((x==NULL) || ...) S
> + memset(x,0, ...);
> +
> +@script:python depends on org@
> +p << r.p;
> +x << r.x;
> +@@
> +
> +msg="%s" % (x)
> +msg_safe=msg.replace("[","@(").replace("]",")")
> +coccilib.org.print_todo(p[0], msg_safe)
> +
> +@script:python depends on report@
> +p << r.p;
> +x << r.x;
> +@@
> +
> +msg="WARNING: *_pool_zalloc should be used for %s, instead of *_pool_alloc/memset" % (x)
> +coccilib.report.print_report(p[0], msg)
> --
> 1.9.1
>
>
On Fri, 31 Jul 2015, Sean O. Stalley wrote:
> Currently the __GFP_ZERO flag is ignored by dma_pool_alloc().
> Make dma_pool_alloc() zero the memory if this flag is set.
>
> Signed-off-by: Sean O. Stalley <[email protected]>
Acked-by: David Rientjes <[email protected]>
This has impacted us as well, and I'm glad to see it fixed.