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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb443e86-2639-4ed5-8d9c-00aa5e200f7e@oracle.com>
Date: Mon, 28 Apr 2025 11:35:12 +0100
From: John Garry <john.g.garry@...cle.com>
To: Caleb Sander Mateos <csander@...estorage.com>,
        Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>,
        Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc: Kanchan Joshi <joshi.k@...sung.com>, linux-nvme@...ts.infradead.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 1/3] dmapool: add NUMA affinity support

On 26/04/2025 03:06, Caleb Sander Mateos wrote:
> From: Keith Busch <kbusch@...nel.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@...nel.org>
> Signed-off-by: Caleb Sander Mateos <csander@...estorage.com>
> Reviewed-by: Jens Axboe <axboe@...nel.dk>
> Reviewed-by: Sagi Grimberg <sagi@...mberg.me>

FWIW,

Reviewed-by: John Garry <john.g.garry@...cle.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);