Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557006 > unrolled thread

[PATCH] xen-netfront: Fix Rx stall during network stress and OOM

Back to article view | Back to linux.kernel

Contents

  [PATCH] xen-netfront: Fix Rx stall during network stress and OOM Vineeth Remanan Pillai <vineethp@amazon.com> - 2017-01-12 00:20 +0100
    Re: [PATCH] xen-netfront: Fix Rx stall during network stress and  OOM David Miller <davem@davemloft.net> - 2017-01-12 21:20 +0100
      Re: [PATCH] xen-netfront: Fix Rx stall during network stress and OOM Vineeth Remanan Pillai <vineethp@amazon.com> - 2017-01-13 00:20 +0100
        Re: [PATCH v2] xen-netfront: Fix Rx stall during network stress and  OOM Juergen Gross <jgross@suse.com> - 2017-01-16 07:30 +0100

#1557006 — [PATCH] xen-netfront: Fix Rx stall during network stress and OOM

During an OOM scenario, request slots could not be created as skb
allocation fails. So the netback cannot pass in packets and netfront
wrongly assumes that there is no more work to be done and it disables
polling. This causes Rx to stall.

Fix is to consider the skb allocation failure as an error and in the
event of this error, re-enable polling so that request slots could be
created when memory is available.

Signed-off-by: Vineeth Remanan Pillai <vineethp@amazon.com>
---
 drivers/net/xen-netfront.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 40f26b6..8275549 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -277,13 +277,14 @@ static struct sk_buff *xennet_alloc_one_rx_buffer(struct netfront_queue *queue)
 }
 
 
-static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
+static int xennet_alloc_rx_buffers(struct netfront_queue *queue)
 {
 	RING_IDX req_prod = queue->rx.req_prod_pvt;
 	int notify;
+	int err = 0;
 
 	if (unlikely(!netif_carrier_ok(queue->info->netdev)))
-		return;
+		return err;
 
 	for (req_prod = queue->rx.req_prod_pvt;
 	     req_prod - queue->rx.rsp_cons < NET_RX_RING_SIZE;
@@ -295,8 +296,10 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
 		struct xen_netif_rx_request *req;
 
 		skb = xennet_alloc_one_rx_buffer(queue);
-		if (!skb)
+		if (!skb) {
+			err = -ENOMEM;
 			break;
+		}
 
 		id = xennet_rxidx(req_prod);
 
@@ -321,9 +324,9 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
 	queue->rx.req_prod_pvt = req_prod;
 
 	/* Not enough requests? Try again later. */
-	if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
+	if (req_prod - queue->rx.sring->rsp_prod < NET_RX_SLOTS_MIN) {
 		mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
-		return;
+		return err;
 	}
 
 	wmb();		/* barrier so backend seens requests */
@@ -331,6 +334,8 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&queue->rx, notify);
 	if (notify)
 		notify_remote_via_irq(queue->rx_irq);
+
+	return err;
 }
 
 static int xennet_open(struct net_device *dev)
@@ -1046,7 +1051,7 @@ static int xennet_poll(struct napi_struct *napi, int budget)
 
 	work_done -= handle_incoming_queue(queue, &rxq);
 
-	xennet_alloc_rx_buffers(queue);
+	err = xennet_alloc_rx_buffers(queue);
 
 	if (work_done < budget) {
 		int more_to_do = 0;
@@ -1054,7 +1059,11 @@ static int xennet_poll(struct napi_struct *napi, int budget)
 		napi_complete(napi);
 
 		RING_FINAL_CHECK_FOR_RESPONSES(&queue->rx, more_to_do);
-		if (more_to_do)
+
+		/* If there is more work to do or could not allocate
+		 * rx buffers, re-enable polling.
+		 */
+		if (more_to_do || err != 0)
 			napi_schedule(napi);
 	}
 
-- 
2.1.2.AMZN

[toc] | [next] | [standalone]

#1557773 — Re: [PATCH] xen-netfront: Fix Rx stall during network stress and OOM

From: Vineeth Remanan Pillai <vineethp@amazon.com>
Date: Wed, 11 Jan 2017 23:17:17 +0000

> @@ -1054,7 +1059,11 @@ static int xennet_poll(struct napi_struct *napi, int budget)
>  		napi_complete(napi);
>  
>  		RING_FINAL_CHECK_FOR_RESPONSES(&queue->rx, more_to_do);
> -		if (more_to_do)
> +
> +		/* If there is more work to do or could not allocate
> +		 * rx buffers, re-enable polling.
> +		 */
> +		if (more_to_do || err != 0)
>  			napi_schedule(napi);

Just polling endlessly in a loop retrying the SKB allocation over and over
again until it succeeds is not very nice behavior.

You already have that refill timer, so please use that to retry instead
of wasting cpu cycles looping in NAPI poll.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1557911

On 01/12/2017 12:17 PM, David Miller wrote:
> From: Vineeth Remanan Pillai <vineethp@amazon.com>
> Date: Wed, 11 Jan 2017 23:17:17 +0000
>
>> @@ -1054,7 +1059,11 @@ static int xennet_poll(struct napi_struct *napi, int budget)
>>   		napi_complete(napi);
>>   
>>   		RING_FINAL_CHECK_FOR_RESPONSES(&queue->rx, more_to_do);
>> -		if (more_to_do)
>> +
>> +		/* If there is more work to do or could not allocate
>> +		 * rx buffers, re-enable polling.
>> +		 */
>> +		if (more_to_do || err != 0)
>>   			napi_schedule(napi);
> Just polling endlessly in a loop retrying the SKB allocation over and over
> again until it succeeds is not very nice behavior.
>
> You already have that refill timer, so please use that to retry instead
> of wasting cpu cycles looping in NAPI poll.
Thanks Dave for the inputs.
On further look, I think I can fix it much simpler by correcting the 
test condition
for minimum slots for pushing requests. Existing test is like this:

<snip>
         /* Not enough requests? Try again later. */
        if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
                 mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
                 return;
         }
</snip>

Actually the above check counts more than the newly created request slots
as it counts from rsp_cons. The actual count should be the difference 
between
new req_prod and old req_prod(in the queue). If skbs cannot be created, 
this
count remains small and hence we would schedule the timer. So the fix 
could be:

         /* Not enough requests? Try again later. */
-       if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
+       if (req_prod - queue->rx.sring->req_prod < NET_RX_SLOTS_MIN) {


I have done some initial testing to verify the fix. Will send out v2 
patch after couple
more round of testing.

Thanks,
Vineeth

[toc] | [prev] | [next] | [standalone]

#1559458 — Re: [PATCH v2] xen-netfront: Fix Rx stall during network stress and OOM

On 13/01/17 18:55, Remanan Pillai wrote:
> From: Vineeth Remanan Pillai <vineethp@amazon.com>
> 
> During an OOM scenario, request slots could not be created as skb
> allocation fails. So the netback cannot pass in packets and netfront
> wrongly assumes that there is no more work to be done and it disables
> polling. This causes Rx to stall.
> 
> The issue is with the retry logic which schedules the timer if the
> created slots are less than NET_RX_SLOTS_MIN. The count of new request
> slots to be pushed are calculated as a difference between new req_prod
> and rsp_cons which could be more than the actual slots, if there are
> unconsumed responses.
> 
> The fix is to calculate the count of newly created slots as the
> difference between new req_prod and old req_prod.
> 
> Signed-off-by: Vineeth Remanan Pillai <vineethp@amazon.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Thanks,

Juergen

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web