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

John Garry
2025-04-28T10:35:40Z

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);