mm/dmapool.c: avoid duplicate memset within dma_pool_alloc
Subsystems (2)
PATCH 1/1 mm/dmapool.c: avoid duplicate memset within dma_pool_alloc
From: Liu Song <[email protected]>
In "dma_direct_alloc", the allocated memory is explicitly set to 0. If use direct alloc, we need to avoid possible duplicate memset in dma_pool_alloc.
--- mm/dmapool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
mm/dmapool.c +2 -1
@@ -21,6 +21,7 @@#include <linux/device.h> #include <linux/dma-mapping.h> +#include <linux/dma-map-ops.h> #include <linux/dmapool.h> #include <linux/kernel.h> #include <linux/list.h>
@@ -372,7 +373,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,#endif spin_unlock_irqrestore (&pool->lock, flags); -if (want_init_on_alloc (mem_flags)) +if (want_init_on_alloc (mem_flags) &&get_dma_ops (pool->dev))memset (retval,0 , pool->size);return retval;
Signature · 1 line
1.8.3.1
Andrew Morton
maintainerQuoted 5 lines · On Thu, 14 Jul 2022 11:25:00 +0800 Liu Song <[email protected]> wrote:
From: Liu Song <[email protected]>
In "
dma_direct_alloc", the allocated memory is explicitly set to 0. If use direct alloc, we need to avoid possible duplicate memset indma_pool_alloc.
I'm having trouble seeing how this change is safe and correct and maintainable. Please describe the code flow more completely?
Quoted 11 lines
mm/dmapool.c +1
-0@@ -21,6 +21,7 @@#include <linux/device.h> #include <linux/dma-mapping.h> +#include <linux/dma-map-ops.h> #include <linux/dmapool.h> #include <linux/kernel.h> #include <linux/list.h>
mm/dmapool.c void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags, · line 377
Quoted 6 lines
mm/dmapool.c +1
-1@@ -372,7 +373,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,#endif spin_unlock_irqrestore (&pool->lock, flags); -if (want_init_on_alloc (mem_flags)) +if (want_init_on_alloc (mem_flags) &&get_dma_ops (pool->dev))memset (retval,0 , pool->size);
That DMAPOOL_DEBUG memset a couple of lines earlier could/should be testing the same condition - there's no point in poisoning an area which we're about to zero out.
Quoted 8 lines
Quoted 5 lines
From: Liu Song <[email protected]>
In "
dma_direct_alloc", the allocated memory is explicitly set to 0. If use direct alloc, we need to avoid possible duplicate memset indma_pool_alloc.I'm having trouble seeing how this change is safe and correct and maintainable. Please describe the code flow more completely?
The following is the code flow,dma_pool_alloc
|--> pool_alloc_page
|--> dma_alloc_coherent
|--> dma_alloc_attrsIn "dma_alloc_attrs", if "dma_alloc_direct" is true, then enter "dma_direct_alloc", and in "dma_direct_alloc", as long as the memory allocation is successful, will execute "memset(ret, 0, size);", which set memory to zero.
Kernel use "dma_go_direct" to determine whether to use direct allocation, which mainly by judging whether "dma_map_ops" exists.
So this patch determines whether direct alloc will be used by judging does "dma_map_ops" exists, thereby avoiding repeated memset.
Quoted 15 lines
Quoted 15 lines
mm/dmapool.c +1
-0@@ -21,6 +21,7 @@#include <linux/device.h> #include <linux/dma-mapping.h> +#include <linux/dma-map-ops.h> #include <linux/dmapool.h> #include <linux/kernel.h> #include <linux/list.h> @@ -372,7 +373,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,#endif spin_unlock_irqrestore (&pool->lock, flags);
Quoted 7 lines
mm/dmapool.c void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags, · line 377
Quoted 3 lines
mm/dmapool.c +1
-1-if (want_init_on_alloc (mem_flags)) +if (want_init_on_alloc (mem_flags) &&get_dma_ops (pool->dev))memset (retval,0 , pool->size);That
DMAPOOL_DEBUGmemset a couple of lines earlier could/should be testing the same condition - there's no point in poisoning an area which we're about to zero out.
If DMAPOOL_DEBUG is configured, its logic is internally self-consistent. If the user needs __GFP_ZERO, the corresponding memory will be set to 0.
This patch can not only consider direct alloc, with config CONFIG_DMA_DECLARE_COHERENT, in "dma_alloc_from_dev_coherent", which will also explicitly memset the allocated memory to 0, so this situation needs to be considered. It is beneficial to the process of calling "dma_pool_zalloc". Next, a V2 patch will be submitted.
Thanks.