All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] sfc: NUMA support
@ 2015-10-28 15:00 Shradha Shah
  2015-10-28 15:01 ` [PATCH net-next 1/4] sfc: use __GFP_NOWARN when allocating RX pages from atomic context Shradha Shah
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Shradha Shah @ 2015-10-28 15:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

This patch series adds support for 
- allocating rx pages local to the interrupt
- setting affinity hint to influence IRQs to be allocated on the
same NUMA node as the one where the card resides.

Alexandra Kossovsky (1):
  sfc: use __GFP_NOWARN when allocating RX pages from atomic context.

Bert Kenward (2):
  sfc: Use cpu_to_mem() to support memoryless nodes
  sfc: set and clear interrupt affinity hints

Daniel Pieczko (1):
  sfc: allocate rx pages on the same node as the interrupt

 drivers/net/ethernet/sfc/efx.c        | 36 +++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/net_driver.h |  3 +++
 drivers/net/ethernet/sfc/rx.c         | 18 +++++++++++++++---
 3 files changed, 54 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/4] sfc: use __GFP_NOWARN when allocating RX pages from atomic context.
  2015-10-28 15:00 [PATCH net-next 0/4] sfc: NUMA support Shradha Shah
@ 2015-10-28 15:01 ` Shradha Shah
  2015-10-28 15:01 ` [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt Shradha Shah
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Shradha Shah @ 2015-10-28 15:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

From: Alexandra Kossovsky <Alexandra.Kossovsky@oktetlabs.ru>

If we fail to allocate a page when in atomic context this is
handled by scheduling a fill in non-atomic context.
As such, a warning is not needed.

Signed-off-by: Shradha Shah <sshah@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index 809ea461..3f0e129 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -163,8 +163,15 @@ static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue, bool atomic)
 	do {
 		page = efx_reuse_page(rx_queue);
 		if (page == NULL) {
+			/* GFP_ATOMIC may fail because of various reasons,
+			 * and we re-schedule rx_fill from non-atomic
+			 * context in such a case.  So, use __GFP_NO_WARN
+			 * in case of atomic.
+			 */
 			page = alloc_pages(__GFP_COLD | __GFP_COMP |
-					   (atomic ? GFP_ATOMIC : GFP_KERNEL),
+					   (atomic ?
+					    (GFP_ATOMIC | __GFP_NOWARN)
+					    : GFP_KERNEL),
 					   efx->rx_buffer_order);
 			if (unlikely(page == NULL))
 				return -ENOMEM;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt
  2015-10-28 15:00 [PATCH net-next 0/4] sfc: NUMA support Shradha Shah
  2015-10-28 15:01 ` [PATCH net-next 1/4] sfc: use __GFP_NOWARN when allocating RX pages from atomic context Shradha Shah
@ 2015-10-28 15:01 ` Shradha Shah
  2015-10-28 15:59   ` Eric Dumazet
  2015-10-28 15:01 ` [PATCH net-next 3/4] sfc: Use cpu_to_mem() to support memoryless nodes Shradha Shah
  2015-10-28 15:02 ` [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints Shradha Shah
  3 siblings, 1 reply; 8+ messages in thread
From: Shradha Shah @ 2015-10-28 15:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

From: Daniel Pieczko <dpieczko@solarflare.com>

When the interrupt servicing a channel is on a NUMA node that is
not local to the device, performance is improved by allocating
rx pages on the node local to the interrupt (remote to the device)

The performance-optimal case, where interrupts and applications
are pinned to CPUs on the same node as the device, is not altered
by this change.

This change gave a 1% improvement in transaction rate using Nginx
with all interrupts and Nginx threads on the node remote to the
device. It also gave a small reduction in round-trip latency,
again with the interrupt and application on a different node to
the device.

Allocating rx pages based on the channel->irq_node value is only
valid for the initial driver-load interrupt affinities; if an
interrupt is moved later, the wrong node may be used for the
allocation.

Signed-off-by: Shradha Shah <sshah@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c        |  1 +
 drivers/net/ethernet/sfc/net_driver.h |  3 +++
 drivers/net/ethernet/sfc/rx.c         | 14 +++++++++-----
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 974637d..89fbd03 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -445,6 +445,7 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
 	channel->efx = efx;
 	channel->channel = i;
 	channel->type = &efx_default_channel_type;
+	channel->irq_node = NUMA_NO_NODE;
 
 	for (j = 0; j < EFX_TXQ_TYPES; j++) {
 		tx_queue = &channel->tx_queue[j];
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index ad56231..0ab9080a 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -419,6 +419,7 @@ enum efx_sync_events_state {
  * @sync_events_state: Current state of sync events on this channel
  * @sync_timestamp_major: Major part of the last ptp sync event
  * @sync_timestamp_minor: Minor part of the last ptp sync event
+ * @irq_node: NUMA node of interrupt
  */
 struct efx_channel {
 	struct efx_nic *efx;
@@ -477,6 +478,8 @@ struct efx_channel {
 	enum efx_sync_events_state sync_events_state;
 	u32 sync_timestamp_major;
 	u32 sync_timestamp_minor;
+
+	int irq_node;
 };
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index 3f0e129..c5ef1e8 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -168,11 +168,15 @@ static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue, bool atomic)
 			 * context in such a case.  So, use __GFP_NO_WARN
 			 * in case of atomic.
 			 */
-			page = alloc_pages(__GFP_COLD | __GFP_COMP |
-					   (atomic ?
-					    (GFP_ATOMIC | __GFP_NOWARN)
-					    : GFP_KERNEL),
-					   efx->rx_buffer_order);
+			struct efx_channel *channel;
+
+			channel = efx_rx_queue_channel(rx_queue);
+			page = alloc_pages_node(channel->irq_node, __GFP_COMP |
+						(atomic ?
+						 (GFP_ATOMIC | __GFP_NOWARN)
+						 : GFP_KERNEL),
+						efx->rx_buffer_order);
+
 			if (unlikely(page == NULL))
 				return -ENOMEM;
 			dma_addr =

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 3/4] sfc: Use cpu_to_mem() to support memoryless nodes
  2015-10-28 15:00 [PATCH net-next 0/4] sfc: NUMA support Shradha Shah
  2015-10-28 15:01 ` [PATCH net-next 1/4] sfc: use __GFP_NOWARN when allocating RX pages from atomic context Shradha Shah
  2015-10-28 15:01 ` [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt Shradha Shah
@ 2015-10-28 15:01 ` Shradha Shah
  2015-10-28 15:02 ` [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints Shradha Shah
  3 siblings, 0 replies; 8+ messages in thread
From: Shradha Shah @ 2015-10-28 15:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

From: Bert Kenward <bkenward@solarflare.com>

With CONFIG_HAVE_MEMORYLESS_NODES cpu_to_node() may return
nodes without memory, which is not a good choice when later
using that to allocate memory. cpu_to_mem() instead provides
the most appropriate NUMA node to allocate from.

Signed-off-by: Shradha Shah <sshah@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c        | 2 +-
 drivers/net/ethernet/sfc/net_driver.h | 4 ++--
 drivers/net/ethernet/sfc/rx.c         | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 89fbd03..84f9e90 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -445,7 +445,7 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
 	channel->efx = efx;
 	channel->channel = i;
 	channel->type = &efx_default_channel_type;
-	channel->irq_node = NUMA_NO_NODE;
+	channel->irq_mem_node = NUMA_NO_NODE;
 
 	for (j = 0; j < EFX_TXQ_TYPES; j++) {
 		tx_queue = &channel->tx_queue[j];
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 0ab9080a..bab6cc0 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -419,7 +419,7 @@ enum efx_sync_events_state {
  * @sync_events_state: Current state of sync events on this channel
  * @sync_timestamp_major: Major part of the last ptp sync event
  * @sync_timestamp_minor: Minor part of the last ptp sync event
- * @irq_node: NUMA node of interrupt
+ * @irq_mem_node: Memory NUMA node of interrupt
  */
 struct efx_channel {
 	struct efx_nic *efx;
@@ -479,7 +479,7 @@ struct efx_channel {
 	u32 sync_timestamp_major;
 	u32 sync_timestamp_minor;
 
-	int irq_node;
+	int irq_mem_node;
 };
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index c5ef1e8..095d1af 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -171,7 +171,8 @@ static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue, bool atomic)
 			struct efx_channel *channel;
 
 			channel = efx_rx_queue_channel(rx_queue);
-			page = alloc_pages_node(channel->irq_node, __GFP_COMP |
+			page = alloc_pages_node(channel->irq_mem_node,
+						__GFP_COMP |
 						(atomic ?
 						 (GFP_ATOMIC | __GFP_NOWARN)
 						 : GFP_KERNEL),

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints
  2015-10-28 15:00 [PATCH net-next 0/4] sfc: NUMA support Shradha Shah
                   ` (2 preceding siblings ...)
  2015-10-28 15:01 ` [PATCH net-next 3/4] sfc: Use cpu_to_mem() to support memoryless nodes Shradha Shah
@ 2015-10-28 15:02 ` Shradha Shah
  2015-10-28 18:15   ` Sergei Shtylyov
  3 siblings, 1 reply; 8+ messages in thread
From: Shradha Shah @ 2015-10-28 15:02 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers

From: Bert Kenward <bkenward@solarflare.com>

Use cpumask_local_spread to provide interrupt affinity hints
for each queue. This will spread interrupts across NUMA local
CPUs first, extending to remote nodes if needed.

Signed-off-by: Shradha Shah <sshah@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 84f9e90..93c4c0e 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1489,6 +1489,30 @@ static int efx_probe_interrupts(struct efx_nic *efx)
 	return 0;
 }
 
+#if defined(CONFIG_SMP)
+static void efx_set_interrupt_affinity(struct efx_nic *efx)
+{
+	struct efx_channel *channel;
+	unsigned int cpu;
+
+	efx_for_each_channel(channel, efx) {
+		cpu = cpumask_local_spread(channel->channel,
+					   pcibus_to_node(efx->pci_dev->bus));
+
+		irq_set_affinity_hint(channel->irq, cpumask_of(cpu));
+		channel->irq_mem_node = cpu_to_mem(cpu);
+	}
+}
+
+static void efx_clear_interrupt_affinity(struct efx_nic *efx)
+{
+	struct efx_channel *channel;
+
+	efx_for_each_channel(channel, efx)
+		irq_set_affinity_hint(channel->irq, NULL);
+}
+#endif /* CONFIG_SMP */
+
 static int efx_soft_enable_interrupts(struct efx_nic *efx)
 {
 	struct efx_channel *channel, *end_channel;
@@ -2932,6 +2956,9 @@ static void efx_pci_remove_main(struct efx_nic *efx)
 	cancel_work_sync(&efx->reset_work);
 
 	efx_disable_interrupts(efx);
+#if defined(CONFIG_SMP)
+	efx_clear_interrupt_affinity(efx);
+#endif
 	efx_nic_fini_interrupt(efx);
 	efx_fini_port(efx);
 	efx->type->fini(efx);
@@ -3081,6 +3108,11 @@ static int efx_pci_probe_main(struct efx_nic *efx)
 	rc = efx_nic_init_interrupt(efx);
 	if (rc)
 		goto fail5;
+
+#if defined(CONFIG_SMP)
+	efx_set_interrupt_affinity(efx);
+#endif
+
 	rc = efx_enable_interrupts(efx);
 	if (rc)
 		goto fail6;
@@ -3088,6 +3120,9 @@ static int efx_pci_probe_main(struct efx_nic *efx)
 	return 0;
 
  fail6:
+#if defined(CONFIG_SMP)
+	efx_clear_interrupt_affinity(efx);
+#endif
 	efx_nic_fini_interrupt(efx);
  fail5:
 	efx_fini_port(efx);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt
  2015-10-28 15:01 ` [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt Shradha Shah
@ 2015-10-28 15:59   ` Eric Dumazet
  2015-11-02 13:17     ` Daniel Pieczko (dpieczko)
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2015-10-28 15:59 UTC (permalink / raw)
  To: Shradha Shah; +Cc: David Miller, netdev, linux-net-drivers

On Wed, 2015-10-28 at 15:01 +0000, Shradha Shah wrote:
> From: Daniel Pieczko <dpieczko@solarflare.com>
> 
> When the interrupt servicing a channel is on a NUMA node that is
> not local to the device, performance is improved by allocating
> rx pages on the node local to the interrupt (remote to the device)
> 
> The performance-optimal case, where interrupts and applications
> are pinned to CPUs on the same node as the device, is not altered
> by this change.
> 
> This change gave a 1% improvement in transaction rate using Nginx
> with all interrupts and Nginx threads on the node remote to the
> device. It also gave a small reduction in round-trip latency,
> again with the interrupt and application on a different node to
> the device.
> 
> Allocating rx pages based on the channel->irq_node value is only
> valid for the initial driver-load interrupt affinities; if an
> interrupt is moved later, the wrong node may be used for the
> allocation.
> 
> Signed-off-by: Shradha Shah <sshah@solarflare.com>
> ---
>  drivers/net/ethernet/sfc/efx.c        |  1 +
>  drivers/net/ethernet/sfc/net_driver.h |  3 +++
>  drivers/net/ethernet/sfc/rx.c         | 14 +++++++++-----
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 974637d..89fbd03 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -445,6 +445,7 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
>  	channel->efx = efx;
>  	channel->channel = i;
>  	channel->type = &efx_default_channel_type;
> +	channel->irq_node = NUMA_NO_NODE;
>  
>  	for (j = 0; j < EFX_TXQ_TYPES; j++) {
>  		tx_queue = &channel->tx_queue[j];
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index ad56231..0ab9080a 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -419,6 +419,7 @@ enum efx_sync_events_state {
>   * @sync_events_state: Current state of sync events on this channel
>   * @sync_timestamp_major: Major part of the last ptp sync event
>   * @sync_timestamp_minor: Minor part of the last ptp sync event
> + * @irq_node: NUMA node of interrupt
>   */
>  struct efx_channel {
>  	struct efx_nic *efx;
> @@ -477,6 +478,8 @@ struct efx_channel {
>  	enum efx_sync_events_state sync_events_state;
>  	u32 sync_timestamp_major;
>  	u32 sync_timestamp_minor;
> +
> +	int irq_node;
>  };
>  
>  #ifdef CONFIG_NET_RX_BUSY_POLL
> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> index 3f0e129..c5ef1e8 100644
> --- a/drivers/net/ethernet/sfc/rx.c
> +++ b/drivers/net/ethernet/sfc/rx.c
> @@ -168,11 +168,15 @@ static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue, bool atomic)
>  			 * context in such a case.  So, use __GFP_NO_WARN
>  			 * in case of atomic.
>  			 */
> -			page = alloc_pages(__GFP_COLD | __GFP_COMP |
> -					   (atomic ?
> -					    (GFP_ATOMIC | __GFP_NOWARN)
> -					    : GFP_KERNEL),
> -					   efx->rx_buffer_order);
> +			struct efx_channel *channel;
> +
> +			channel = efx_rx_queue_channel(rx_queue);
> +			page = alloc_pages_node(channel->irq_node, __GFP_COMP |
> +						(atomic ?
> +						 (GFP_ATOMIC | __GFP_NOWARN)
> +						 : GFP_KERNEL),
> +						efx->rx_buffer_order);
> +
>  			if (unlikely(page == NULL))
>  				return -ENOMEM;
>  			dma_addr =
> 


Sorry, I do not understand this patch, and why the following one is not
squashed on this one.

irq_node is always NUMA_NO_NODE (in this patch)

So you claim a 1% improvement, switching from alloc_pages(...) to
alloc_pages_node(NUMA_NO_NODE, ...) ???

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints
  2015-10-28 15:02 ` [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints Shradha Shah
@ 2015-10-28 18:15   ` Sergei Shtylyov
  0 siblings, 0 replies; 8+ messages in thread
From: Sergei Shtylyov @ 2015-10-28 18:15 UTC (permalink / raw)
  To: Shradha Shah, David Miller; +Cc: netdev, linux-net-drivers

Hello.

On 10/28/2015 06:02 PM, Shradha Shah wrote:

> From: Bert Kenward <bkenward@solarflare.com>
>
> Use cpumask_local_spread to provide interrupt affinity hints
> for each queue. This will spread interrupts across NUMA local
> CPUs first, extending to remote nodes if needed.
>
> Signed-off-by: Shradha Shah <sshah@solarflare.com>
> ---
>   drivers/net/ethernet/sfc/efx.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
>
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 84f9e90..93c4c0e 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -1489,6 +1489,30 @@ static int efx_probe_interrupts(struct efx_nic *efx)
>   	return 0;
>   }
>
> +#if defined(CONFIG_SMP)
> +static void efx_set_interrupt_affinity(struct efx_nic *efx)
> +{
> +	struct efx_channel *channel;
> +	unsigned int cpu;
> +
> +	efx_for_each_channel(channel, efx) {
> +		cpu = cpumask_local_spread(channel->channel,
> +					   pcibus_to_node(efx->pci_dev->bus));
> +
> +		irq_set_affinity_hint(channel->irq, cpumask_of(cpu));
> +		channel->irq_mem_node = cpu_to_mem(cpu);
> +	}
> +}
> +
> +static void efx_clear_interrupt_affinity(struct efx_nic *efx)
> +{
> +	struct efx_channel *channel;
> +
> +	efx_for_each_channel(channel, efx)
> +		irq_set_affinity_hint(channel->irq, NULL);
> +}
> +#endif /* CONFIG_SMP */
> +
>   static int efx_soft_enable_interrupts(struct efx_nic *efx)
>   {
>   	struct efx_channel *channel, *end_channel;
> @@ -2932,6 +2956,9 @@ static void efx_pci_remove_main(struct efx_nic *efx)
>   	cancel_work_sync(&efx->reset_work);
>
>   	efx_disable_interrupts(efx);
> +#if defined(CONFIG_SMP)
> +	efx_clear_interrupt_affinity(efx);
> +#endif

    Please just define empty function for !SMP case instead of the ugly 
#ifdef'fery in the functiojn bodies.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt
  2015-10-28 15:59   ` Eric Dumazet
@ 2015-11-02 13:17     ` Daniel Pieczko (dpieczko)
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Pieczko (dpieczko) @ 2015-11-02 13:17 UTC (permalink / raw)
  To: Eric Dumazet, Shradha Shah; +Cc: David Miller, netdev, linux-net-drivers

On 28/10/15 15:59, Eric Dumazet wrote:
> On Wed, 2015-10-28 at 15:01 +0000, Shradha Shah wrote:
>> From: Daniel Pieczko <dpieczko@solarflare.com>
>>
>> When the interrupt servicing a channel is on a NUMA node that is
>> not local to the device, performance is improved by allocating
>> rx pages on the node local to the interrupt (remote to the device)
>>
>> The performance-optimal case, where interrupts and applications
>> are pinned to CPUs on the same node as the device, is not altered
>> by this change.
>>
>> This change gave a 1% improvement in transaction rate using Nginx
>> with all interrupts and Nginx threads on the node remote to the
>> device. It also gave a small reduction in round-trip latency,
>> again with the interrupt and application on a different node to
>> the device.
>>
>> Allocating rx pages based on the channel->irq_node value is only
>> valid for the initial driver-load interrupt affinities; if an
>> interrupt is moved later, the wrong node may be used for the
>> allocation.
>>
>> Signed-off-by: Shradha Shah <sshah@solarflare.com>
>> ---
>>  drivers/net/ethernet/sfc/efx.c        |  1 +
>>  drivers/net/ethernet/sfc/net_driver.h |  3 +++
>>  drivers/net/ethernet/sfc/rx.c         | 14 +++++++++-----
>>  3 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
>> index 974637d..89fbd03 100644
>> --- a/drivers/net/ethernet/sfc/efx.c
>> +++ b/drivers/net/ethernet/sfc/efx.c
>> @@ -445,6 +445,7 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
>>  	channel->efx = efx;
>>  	channel->channel = i;
>>  	channel->type = &efx_default_channel_type;
>> +	channel->irq_node = NUMA_NO_NODE;
>>  
>>  	for (j = 0; j < EFX_TXQ_TYPES; j++) {
>>  		tx_queue = &channel->tx_queue[j];
>> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
>> index ad56231..0ab9080a 100644
>> --- a/drivers/net/ethernet/sfc/net_driver.h
>> +++ b/drivers/net/ethernet/sfc/net_driver.h
>> @@ -419,6 +419,7 @@ enum efx_sync_events_state {
>>   * @sync_events_state: Current state of sync events on this channel
>>   * @sync_timestamp_major: Major part of the last ptp sync event
>>   * @sync_timestamp_minor: Minor part of the last ptp sync event
>> + * @irq_node: NUMA node of interrupt
>>   */
>>  struct efx_channel {
>>  	struct efx_nic *efx;
>> @@ -477,6 +478,8 @@ struct efx_channel {
>>  	enum efx_sync_events_state sync_events_state;
>>  	u32 sync_timestamp_major;
>>  	u32 sync_timestamp_minor;
>> +
>> +	int irq_node;
>>  };
>>  
>>  #ifdef CONFIG_NET_RX_BUSY_POLL
>> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
>> index 3f0e129..c5ef1e8 100644
>> --- a/drivers/net/ethernet/sfc/rx.c
>> +++ b/drivers/net/ethernet/sfc/rx.c
>> @@ -168,11 +168,15 @@ static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue, bool atomic)
>>  			 * context in such a case.  So, use __GFP_NO_WARN
>>  			 * in case of atomic.
>>  			 */
>> -			page = alloc_pages(__GFP_COLD | __GFP_COMP |
>> -					   (atomic ?
>> -					    (GFP_ATOMIC | __GFP_NOWARN)
>> -					    : GFP_KERNEL),
>> -					   efx->rx_buffer_order);
>> +			struct efx_channel *channel;
>> +
>> +			channel = efx_rx_queue_channel(rx_queue);
>> +			page = alloc_pages_node(channel->irq_node, __GFP_COMP |
>> +						(atomic ?
>> +						 (GFP_ATOMIC | __GFP_NOWARN)
>> +						 : GFP_KERNEL),
>> +						efx->rx_buffer_order);
>> +
>>  			if (unlikely(page == NULL))
>>  				return -ENOMEM;
>>  			dma_addr =
>>
>
> Sorry, I do not understand this patch, and why the following one is not
> squashed on this one.
>
> irq_node is always NUMA_NO_NODE (in this patch)
>
> So you claim a 1% improvement, switching from alloc_pages(...) to
> alloc_pages_node(NUMA_NO_NODE, ...) ???
>

You're correct that this doesn't make sense as it is.  There is something missing in this patch (channel->irq_node should be set) and also changing the order of some patches could make this clearer.  The series will need to be resent.


Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-11-02 13:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-28 15:00 [PATCH net-next 0/4] sfc: NUMA support Shradha Shah
2015-10-28 15:01 ` [PATCH net-next 1/4] sfc: use __GFP_NOWARN when allocating RX pages from atomic context Shradha Shah
2015-10-28 15:01 ` [PATCH net-next 2/4] sfc: allocate rx pages on the same node as the interrupt Shradha Shah
2015-10-28 15:59   ` Eric Dumazet
2015-11-02 13:17     ` Daniel Pieczko (dpieczko)
2015-10-28 15:01 ` [PATCH net-next 3/4] sfc: Use cpu_to_mem() to support memoryless nodes Shradha Shah
2015-10-28 15:02 ` [PATCH net-next 4/4] sfc: set and clear interrupt affinity hints Shradha Shah
2015-10-28 18:15   ` Sergei Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.