diff mbox series

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

Message ID 20240723100342.54379-2-philip.cox@canonical.com
State New
Headers show
Series aws: Backport CMA pool per numa functionality for 22.04 and 20.04 | expand

Commit Message

Philip Cox July 23, 2024, 10:03 a.m. UTC
From: Shay Agroskin <shayagr@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@amazon.com>
Link: https://lore.kernel.org/r/20240528170912.1204417-1-shayagr@amazon.com
Signed-off-by: Jakub Kicinski <kuba@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@canonical.com>
---
 drivers/net/ethernet/amazon/ena/ena_com.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Comments

Manuel Diewald July 24, 2024, 10:11 a.m. UTC | #1
On Tue, Jul 23, 2024 at 01:03:42PM +0300, Philip Cox wrote:
> From: Shay Agroskin <shayagr@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@amazon.com>
> Link: https://lore.kernel.org/r/20240528170912.1204417-1-shayagr@amazon.com
> Signed-off-by: Jakub Kicinski <kuba@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@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@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

See inline comment.
diff mbox series

Patch

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