Re: [PATCH v6 1/3] dmapool: add NUMA affinity support

[John Garry <john.g.garry@oracle.com>]

On 26/04/2025 03:06, Caleb Sander Mateos wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Introduce dma_pool_create_node(), like dma_pool_create() but taking an
> additional NUMA node argument. Allocate struct dma_pool on the desired
> node, and store the node on dma_pool for allocating struct dma_page.
> Make dma_pool_create() an alias for dma_pool_create_node() with node set
> to NUMA_NO_NODE.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> Reviewed-by: Jens Axboe <axboe@kernel.dk>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

FWIW,

Reviewed-by: John Garry <john.g.garry@oracle.com>

But a comment below.


>   
>   static struct dma_page *pool_alloc_page(struct dma_pool *pool, gfp_t mem_flags)
>   {
>   	struct dma_page *page;
>   
> -	page = kmalloc(sizeof(*page), mem_flags);
> +	page = kmalloc_node(sizeof(*page), mem_flags, pool->node);
>   	if (!page)
>   		return NULL;

For pool->node != NUMA_NO_NODE, pool->node == numa_node_id(), right? I
mean, aren't we on a CPU in pool->node here? And then - by default - we 
try to allocate from the closest node to current CPU anyway (when not 
specified), I think - that seems to have changed, when checking the 
allocator code.

>   
>   	page->vaddr = dma_alloc_coherent(pool->dev, pool->allocation,
>   					 &page->dma, mem_flags);