NACK/Cmnt: [PATCH 1/1] [n:linux-aws] net: ena: Fix redundant device NUMA node override

[Manuel Diewald]

On Tue, Jul 23, 2024 at 01:03:42PM +0300, Philip Cox wrote:
> From: Shay Agroskin <shayagr at amazon.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2067516
> 
> The driver overrides the NUMA node id of the device regardless of
> whether it knows its correct value (often setting it to -1 even though
> the node id is advertised in 'struct device'). This can lead to
> suboptimal configurations.
> 
> This patch fixes this behavior and makes the shared memory allocation
> functions use the NUMA node id advertised by the underlying device.
> 
> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
> Signed-off-by: Shay Agroskin <shayagr at amazon.com>
> Link: https://lore.kernel.org/r/20240528170912.1204417-1-shayagr@amazon.com
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (backported from commit 2dc8b1e7177d4f49f492ce648440caf2de0c3616)
> [philcox: context changes in ena_com_init_io_cq() and ena_com_init_io_sq()]
> Signed-off-by: Philip Cox <philip.cox at canonical.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_com.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index 4db689372980..3a5b307d5fb8 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -320,7 +320,6 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
>  			      struct ena_com_io_sq *io_sq)
>  {
>  	size_t size;
> -	int dev_node = 0;
>  
>  	memset(&io_sq->desc_addr, 0x0, sizeof(io_sq->desc_addr));
>  
> @@ -333,13 +332,10 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
>  	size = io_sq->desc_entry_size * io_sq->q_depth;
>  
>  	if (io_sq->mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST) {
> -		dev_node = dev_to_node(ena_dev->dmadev);
> -		set_dev_node(ena_dev->dmadev, ctx->numa_node);
>  		io_sq->desc_addr.virt_addr =
>  			dma_alloc_coherent(ena_dev->dmadev, size,
>  					   &io_sq->desc_addr.phys_addr,
>  					   GFP_KERNEL);
> -		set_dev_node(ena_dev->dmadev, dev_node);
>  		if (!io_sq->desc_addr.virt_addr) {
>  			io_sq->desc_addr.virt_addr =
>  				dma_alloc_coherent(ena_dev->dmadev, size,
> @@ -365,11 +361,9 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
>  		size = (size_t)io_sq->bounce_buf_ctrl.buffer_size *
>  			io_sq->bounce_buf_ctrl.buffers_num;
>  
> -		dev_node = dev_to_node(ena_dev->dmadev);
> -		set_dev_node(ena_dev->dmadev, ctx->numa_node);
>  		io_sq->bounce_buf_ctrl.base_buffer =
>  			devm_kzalloc(ena_dev->dmadev, size, GFP_KERNEL);
> -		set_dev_node(ena_dev->dmadev, dev_node);
> +		io_sq->bounce_buf_ctrl.base_buffer = devm_kzalloc(ena_dev->dmadev, size, GFP_KERNEL);

I think this duplicate line addition needs to be removed from the patch.

>  		if (!io_sq->bounce_buf_ctrl.base_buffer)
>  			io_sq->bounce_buf_ctrl.base_buffer =
>  				devm_kzalloc(ena_dev->dmadev, size, GFP_KERNEL);
> @@ -410,7 +404,6 @@ static int ena_com_init_io_cq(struct ena_com_dev *ena_dev,
>  			      struct ena_com_io_cq *io_cq)
>  {
>  	size_t size;
> -	int prev_node = 0;
>  
>  	memset(&io_cq->cdesc_addr, 0x0, sizeof(io_cq->cdesc_addr));
>  
> @@ -422,12 +415,9 @@ static int ena_com_init_io_cq(struct ena_com_dev *ena_dev,
>  
>  	size = io_cq->cdesc_entry_size_in_bytes * io_cq->q_depth;
>  
> -	prev_node = dev_to_node(ena_dev->dmadev);
> -	set_dev_node(ena_dev->dmadev, ctx->numa_node);
>  	io_cq->cdesc_addr.virt_addr =
>  		dma_alloc_coherent(ena_dev->dmadev, size,
>  				   &io_cq->cdesc_addr.phys_addr, GFP_KERNEL);
> -	set_dev_node(ena_dev->dmadev, prev_node);
>  	if (!io_cq->cdesc_addr.virt_addr) {
>  		io_cq->cdesc_addr.virt_addr =
>  			dma_alloc_coherent(ena_dev->dmadev, size,
> -- 
> 2.34.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

See inline comment.

-- 
 Manuel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240724/bcafc1a8/attachment.sig>