linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ena: Speed up initialization 90x by reducing poll delays
@ 2020-02-29  0:28 Josh Triplett
  2020-03-02 23:16 ` Machulsky, Zorik
  0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2020-02-29  0:28 UTC (permalink / raw)
  To: Netanel Belgazal, Arthur Kiyanovski, Guy Tzalik, Saeed Bishara,
	Zorik Machulsky
  Cc: netdev, linux-kernel

Before initializing completion queue interrupts, the ena driver uses
polling to wait for responses on the admin command queue. The ena driver
waits 5ms between polls, but the hardware has generally finished long
before that. Reduce the poll time to 10us.

On a c5.12xlarge, this improves ena initialization time from 173.6ms to
1.920ms, an improvement of more than 90x. This improves server boot time
and time to network bringup.

Before:
[    0.531722] calling  ena_init+0x0/0x63 @ 1
[    0.531722] ena: Elastic Network Adapter (ENA) v2.1.0K
[    0.531751] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
[    0.531946] PCI Interrupt Link [LNKD] enabled at IRQ 11
[    0.547425] ena: ena device version: 0.10
[    0.547427] ena: ena controller version: 0.0.1 implementation version 1
[    0.709497] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
[    0.709508] initcall ena_init+0x0/0x63 returned 0 after 173616 usecs

After:
[    0.526965] calling  ena_init+0x0/0x63 @ 1
[    0.526966] ena: Elastic Network Adapter (ENA) v2.1.0K
[    0.527056] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
[    0.527196] PCI Interrupt Link [LNKD] enabled at IRQ 11
[    0.527211] ena: ena device version: 0.10
[    0.527212] ena: ena controller version: 0.0.1 implementation version 1
[    0.528925] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
[    0.528934] initcall ena_init+0x0/0x63 returned 0 after 1920 usecs

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 drivers/net/ethernet/amazon/ena/ena_com.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 1fb58f9ad80b..203b2130d707 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -62,7 +62,7 @@
 
 #define ENA_REGS_ADMIN_INTR_MASK 1
 
-#define ENA_POLL_MS	5
+#define ENA_POLL_US	10
 
 /*****************************************************************************/
 /*****************************************************************************/
@@ -572,7 +572,7 @@ static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
 			goto err;
 		}
 
-		msleep(ENA_POLL_MS);
+		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
 	}
 
 	if (unlikely(comp_ctx->status == ENA_CMD_ABORTED)) {
@@ -943,12 +943,13 @@ static void ena_com_io_queue_free(struct ena_com_dev *ena_dev,
 static int wait_for_reset_state(struct ena_com_dev *ena_dev, u32 timeout,
 				u16 exp_state)
 {
-	u32 val, i;
+	u32 val;
+	unsigned long timeout_jiffies;
 
-	/* Convert timeout from resolution of 100ms to ENA_POLL_MS */
-	timeout = (timeout * 100) / ENA_POLL_MS;
+	/* Convert timeout from resolution of 100ms */
+	timeout_jiffies = jiffies + msecs_to_jiffies(timeout * 100);
 
-	for (i = 0; i < timeout; i++) {
+	while (1) {
 		val = ena_com_reg_bar_read32(ena_dev, ENA_REGS_DEV_STS_OFF);
 
 		if (unlikely(val == ENA_MMIO_READ_TIMEOUT)) {
@@ -960,10 +961,11 @@ static int wait_for_reset_state(struct ena_com_dev *ena_dev, u32 timeout,
 			exp_state)
 			return 0;
 
-		msleep(ENA_POLL_MS);
-	}
+		if (time_is_before_jiffies(timeout_jiffies))
+			return -ETIME;
 
-	return -ETIME;
+		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
+	}
 }
 
 static bool ena_com_check_supported_feature_id(struct ena_com_dev *ena_dev,
@@ -1458,7 +1460,7 @@ void ena_com_wait_for_abort_completion(struct ena_com_dev *ena_dev)
 	spin_lock_irqsave(&admin_queue->q_lock, flags);
 	while (atomic_read(&admin_queue->outstanding_cmds) != 0) {
 		spin_unlock_irqrestore(&admin_queue->q_lock, flags);
-		msleep(ENA_POLL_MS);
+		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
 		spin_lock_irqsave(&admin_queue->q_lock, flags);
 	}
 	spin_unlock_irqrestore(&admin_queue->q_lock, flags);
-- 
2.25.1


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

* Re: [PATCH] ena: Speed up initialization 90x by reducing poll delays
  2020-02-29  0:28 [PATCH] ena: Speed up initialization 90x by reducing poll delays Josh Triplett
@ 2020-03-02 23:16 ` Machulsky, Zorik
  2020-03-02 23:53   ` Jakub Kicinski
  2020-03-03  0:39   ` Josh Triplett
  0 siblings, 2 replies; 6+ messages in thread
From: Machulsky, Zorik @ 2020-03-02 23:16 UTC (permalink / raw)
  To: Josh Triplett, Belgazal, Netanel, Kiyanovski, Arthur, Tzalik,
	Guy, Bshara, Saeed
  Cc: netdev, linux-kernel


On 2/28/20, 4:29 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:

    Before initializing completion queue interrupts, the ena driver uses
    polling to wait for responses on the admin command queue. The ena driver
    waits 5ms between polls, but the hardware has generally finished long
    before that. Reduce the poll time to 10us.
    
    On a c5.12xlarge, this improves ena initialization time from 173.6ms to
    1.920ms, an improvement of more than 90x. This improves server boot time
    and time to network bringup.
 
Thanks Josh,
We agree that polling rate should be increased, but prefer not to do it aggressively and blindly.
For example linear backoff approach might be a better choice. Please let us re-work a little this 
patch and bring it to review. Thanks!      
   
    Before:
    [    0.531722] calling  ena_init+0x0/0x63 @ 1
    [    0.531722] ena: Elastic Network Adapter (ENA) v2.1.0K
    [    0.531751] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
    [    0.531946] PCI Interrupt Link [LNKD] enabled at IRQ 11
    [    0.547425] ena: ena device version: 0.10
    [    0.547427] ena: ena controller version: 0.0.1 implementation version 1
    [    0.709497] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
    [    0.709508] initcall ena_init+0x0/0x63 returned 0 after 173616 usecs
    
    After:
    [    0.526965] calling  ena_init+0x0/0x63 @ 1
    [    0.526966] ena: Elastic Network Adapter (ENA) v2.1.0K
    [    0.527056] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
    [    0.527196] PCI Interrupt Link [LNKD] enabled at IRQ 11
    [    0.527211] ena: ena device version: 0.10
    [    0.527212] ena: ena controller version: 0.0.1 implementation version 1
    [    0.528925] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
    [    0.528934] initcall ena_init+0x0/0x63 returned 0 after 1920 usecs
    
    Signed-off-by: Josh Triplett <josh@joshtriplett.org>
    ---
     drivers/net/ethernet/amazon/ena/ena_com.c | 22 ++++++++++++----------
     1 file changed, 12 insertions(+), 10 deletions(-)
    
    diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
    index 1fb58f9ad80b..203b2130d707 100644
    --- a/drivers/net/ethernet/amazon/ena/ena_com.c
    +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
    @@ -62,7 +62,7 @@
     
     #define ENA_REGS_ADMIN_INTR_MASK 1
     
    -#define ENA_POLL_MS	5
    +#define ENA_POLL_US	10
     
     /*****************************************************************************/
     /*****************************************************************************/
    @@ -572,7 +572,7 @@ static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
     			goto err;
     		}
     
    -		msleep(ENA_POLL_MS);
    +		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
     	}
     
     	if (unlikely(comp_ctx->status == ENA_CMD_ABORTED)) {
    @@ -943,12 +943,13 @@ static void ena_com_io_queue_free(struct ena_com_dev *ena_dev,
     static int wait_for_reset_state(struct ena_com_dev *ena_dev, u32 timeout,
     				u16 exp_state)
     {
    -	u32 val, i;
    +	u32 val;
    +	unsigned long timeout_jiffies;
     
    -	/* Convert timeout from resolution of 100ms to ENA_POLL_MS */
    -	timeout = (timeout * 100) / ENA_POLL_MS;
    +	/* Convert timeout from resolution of 100ms */
    +	timeout_jiffies = jiffies + msecs_to_jiffies(timeout * 100);
     
    -	for (i = 0; i < timeout; i++) {
    +	while (1) {
     		val = ena_com_reg_bar_read32(ena_dev, ENA_REGS_DEV_STS_OFF);
     
     		if (unlikely(val == ENA_MMIO_READ_TIMEOUT)) {
    @@ -960,10 +961,11 @@ static int wait_for_reset_state(struct ena_com_dev *ena_dev, u32 timeout,
     			exp_state)
     			return 0;
     
    -		msleep(ENA_POLL_MS);
    -	}
    +		if (time_is_before_jiffies(timeout_jiffies))
    +			return -ETIME;
     
    -	return -ETIME;
    +		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
    +	}
     }
     
     static bool ena_com_check_supported_feature_id(struct ena_com_dev *ena_dev,
    @@ -1458,7 +1460,7 @@ void ena_com_wait_for_abort_completion(struct ena_com_dev *ena_dev)
     	spin_lock_irqsave(&admin_queue->q_lock, flags);
     	while (atomic_read(&admin_queue->outstanding_cmds) != 0) {
     		spin_unlock_irqrestore(&admin_queue->q_lock, flags);
    -		msleep(ENA_POLL_MS);
    +		usleep_range(ENA_POLL_US, 2 * ENA_POLL_US);
     		spin_lock_irqsave(&admin_queue->q_lock, flags);
     	}
     	spin_unlock_irqrestore(&admin_queue->q_lock, flags);
    -- 
    2.25.1
    
    


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

* Re: [PATCH] ena: Speed up initialization 90x by reducing poll delays
  2020-03-02 23:16 ` Machulsky, Zorik
@ 2020-03-02 23:53   ` Jakub Kicinski
  2020-03-03  0:43     ` Machulsky, Zorik
  2020-03-03  0:39   ` Josh Triplett
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2020-03-02 23:53 UTC (permalink / raw)
  To: Machulsky, Zorik
  Cc: Josh Triplett, Belgazal, Netanel, Kiyanovski, Arthur, Tzalik,
	Guy, Bshara, Saeed, netdev, linux-kernel

On Mon, 2 Mar 2020 23:16:32 +0000 Machulsky, Zorik wrote:
> On 2/28/20, 4:29 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:
> 
>     Before initializing completion queue interrupts, the ena driver uses
>     polling to wait for responses on the admin command queue. The ena driver
>     waits 5ms between polls, but the hardware has generally finished long
>     before that. Reduce the poll time to 10us.
>     
>     On a c5.12xlarge, this improves ena initialization time from 173.6ms to
>     1.920ms, an improvement of more than 90x. This improves server boot time
>     and time to network bringup.
>  
> Thanks Josh,
> We agree that polling rate should be increased, but prefer not to do
> it aggressively and blindly. For example linear backoff approach
> might be a better choice. Please let us re-work a little this patch
> and bring it to review. Thanks!  

Up to Josh if this is fine with him, but in my experience "let us rework
your patch behind the close doors" is not the response open source
contributors are expecting.

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

* Re: [PATCH] ena: Speed up initialization 90x by reducing poll delays
  2020-03-02 23:16 ` Machulsky, Zorik
  2020-03-02 23:53   ` Jakub Kicinski
@ 2020-03-03  0:39   ` Josh Triplett
  2020-03-03  0:53     ` Machulsky, Zorik
  1 sibling, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2020-03-03  0:39 UTC (permalink / raw)
  To: Machulsky, Zorik
  Cc: Belgazal, Netanel, Kiyanovski, Arthur, Tzalik, Guy, Bshara,
	Saeed, netdev, linux-kernel

On Mon, Mar 02, 2020 at 11:16:32PM +0000, Machulsky, Zorik wrote:
> 
> On 2/28/20, 4:29 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:
> 
>     Before initializing completion queue interrupts, the ena driver uses
>     polling to wait for responses on the admin command queue. The ena driver
>     waits 5ms between polls, but the hardware has generally finished long
>     before that. Reduce the poll time to 10us.
>     
>     On a c5.12xlarge, this improves ena initialization time from 173.6ms to
>     1.920ms, an improvement of more than 90x. This improves server boot time
>     and time to network bringup.
>  
> Thanks Josh,
> We agree that polling rate should be increased, but prefer not to do it aggressively and blindly.
> For example linear backoff approach might be a better choice. Please let us re-work a little this 
> patch and bring it to review. Thanks!

That's fine, as long as it has the same net improvement on boot time.

I'd appreciate the opportunity to test any alternate approach you might
have.

(Also, as long as you're working on this, you might wish to make a
similar change to the EFA driver, and to the FreeBSD drivers.)

>     Before:
>     [    0.531722] calling  ena_init+0x0/0x63 @ 1
>     [    0.531722] ena: Elastic Network Adapter (ENA) v2.1.0K
>     [    0.531751] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
>     [    0.531946] PCI Interrupt Link [LNKD] enabled at IRQ 11
>     [    0.547425] ena: ena device version: 0.10
>     [    0.547427] ena: ena controller version: 0.0.1 implementation version 1
>     [    0.709497] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
>     [    0.709508] initcall ena_init+0x0/0x63 returned 0 after 173616 usecs
>     
>     After:
>     [    0.526965] calling  ena_init+0x0/0x63 @ 1
>     [    0.526966] ena: Elastic Network Adapter (ENA) v2.1.0K
>     [    0.527056] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
>     [    0.527196] PCI Interrupt Link [LNKD] enabled at IRQ 11
>     [    0.527211] ena: ena device version: 0.10
>     [    0.527212] ena: ena controller version: 0.0.1 implementation version 1
>     [    0.528925] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
>     [    0.528934] initcall ena_init+0x0/0x63 returned 0 after 1920 usecs

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

* Re: [PATCH] ena: Speed up initialization 90x by reducing poll delays
  2020-03-02 23:53   ` Jakub Kicinski
@ 2020-03-03  0:43     ` Machulsky, Zorik
  0 siblings, 0 replies; 6+ messages in thread
From: Machulsky, Zorik @ 2020-03-03  0:43 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Josh Triplett, Belgazal, Netanel, Kiyanovski, Arthur, Tzalik,
	Guy, Bshara, Saeed, netdev, linux-kernel



On 3/2/20, 3:54 PM, "Jakub Kicinski" <kuba@kernel.org> wrote:

        
    
    On Mon, 2 Mar 2020 23:16:32 +0000 Machulsky, Zorik wrote:
    > On 2/28/20, 4:29 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:
    >
    >     Before initializing completion queue interrupts, the ena driver uses
    >     polling to wait for responses on the admin command queue. The ena driver
    >     waits 5ms between polls, but the hardware has generally finished long
    >     before that. Reduce the poll time to 10us.
    >
    >     On a c5.12xlarge, this improves ena initialization time from 173.6ms to
    >     1.920ms, an improvement of more than 90x. This improves server boot time
    >     and time to network bringup.
    >
    > Thanks Josh,
    > We agree that polling rate should be increased, but prefer not to do
    > it aggressively and blindly. For example linear backoff approach
    > might be a better choice. Please let us re-work a little this patch
    > and bring it to review. Thanks!
    
    Up to Josh if this is fine with him, but in my experience "let us rework
    your patch behind the close doors" is not the response open source
    contributors are expecting.

Not sure I'm following what you mean by "behind the close door". Everything is open here.
I offered that ENA folks would take it further, because such change require  (in addition to 
Implementation change that we propose) a careful  testing with different platforms and 
instance types. Having said that, Josh, if you would like to take care of it, we will gladly help.
And thank you again for catching this!   


    


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

* Re: [PATCH] ena: Speed up initialization 90x by reducing poll delays
  2020-03-03  0:39   ` Josh Triplett
@ 2020-03-03  0:53     ` Machulsky, Zorik
  0 siblings, 0 replies; 6+ messages in thread
From: Machulsky, Zorik @ 2020-03-03  0:53 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Belgazal, Netanel, Kiyanovski, Arthur, Tzalik, Guy, Bshara,
	Saeed, netdev, linux-kernel



On 3/2/20, 4:40 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:
    
    
    On Mon, Mar 02, 2020 at 11:16:32PM +0000, Machulsky, Zorik wrote:
    >
    > On 2/28/20, 4:29 PM, "Josh Triplett" <josh@joshtriplett.org> wrote:
    >
    >     Before initializing completion queue interrupts, the ena driver uses
    >     polling to wait for responses on the admin command queue. The ena driver
    >     waits 5ms between polls, but the hardware has generally finished long
    >     before that. Reduce the poll time to 10us.
    >
    >     On a c5.12xlarge, this improves ena initialization time from 173.6ms to
    >     1.920ms, an improvement of more than 90x. This improves server boot time
    >     and time to network bringup.
    >
    > Thanks Josh,
    > We agree that polling rate should be increased, but prefer not to do it aggressively and blindly.
    > For example linear backoff approach might be a better choice. Please let us re-work a little this
    > patch and bring it to review. Thanks!
    
    That's fine, as long as it has the same net improvement on boot time.
    
    I'd appreciate the opportunity to test any alternate approach you might
    have.
    
    (Also, as long as you're working on this, you might wish to make a
    similar change to the EFA driver, and to the FreeBSD drivers.) 

Absolutely! Already forwarded this to the owners of these drivers.  Thanks!
    
    >     Before:
    >     [    0.531722] calling  ena_init+0x0/0x63 @ 1
    >     [    0.531722] ena: Elastic Network Adapter (ENA) v2.1.0K
    >     [    0.531751] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
    >     [    0.531946] PCI Interrupt Link [LNKD] enabled at IRQ 11
    >     [    0.547425] ena: ena device version: 0.10
    >     [    0.547427] ena: ena controller version: 0.0.1 implementation version 1
    >     [    0.709497] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
    >     [    0.709508] initcall ena_init+0x0/0x63 returned 0 after 173616 usecs
    >
    >     After:
    >     [    0.526965] calling  ena_init+0x0/0x63 @ 1
    >     [    0.526966] ena: Elastic Network Adapter (ENA) v2.1.0K
    >     [    0.527056] ena 0000:00:05.0: Elastic Network Adapter (ENA) v2.1.0K
    >     [    0.527196] PCI Interrupt Link [LNKD] enabled at IRQ 11
    >     [    0.527211] ena: ena device version: 0.10
    >     [    0.527212] ena: ena controller version: 0.0.1 implementation version 1
    >     [    0.528925] ena 0000:00:05.0: Elastic Network Adapter (ENA) found at mem febf4000, mac addr 06:c4:22:0e:dc:da, Placement policy: Low Latency
    >     [    0.528934] initcall ena_init+0x0/0x63 returned 0 after 1920 usecs
    


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

end of thread, other threads:[~2020-03-03  0:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-29  0:28 [PATCH] ena: Speed up initialization 90x by reducing poll delays Josh Triplett
2020-03-02 23:16 ` Machulsky, Zorik
2020-03-02 23:53   ` Jakub Kicinski
2020-03-03  0:43     ` Machulsky, Zorik
2020-03-03  0:39   ` Josh Triplett
2020-03-03  0:53     ` Machulsky, Zorik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).