diff options
author | Zoltan Kiss <zoltan.kiss@citrix.com> | 2014-02-04 14:54:37 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-02-05 19:24:08 -0500 |
commit | 9ab9831b4c59d379a17f92a6157c7e921811dea2 (patch) | |
tree | db1279d7fcd1847180cc22dc15a809de048509f8 /drivers/net/xen-netback/netback.c | |
parent | bce3ea81d5b2a33ed0e275d58a45f10ce95cbd50 (diff) |
xen-netback: Fix Rx stall due to race condition
The recent patch to fix receive side flow control
(11b57f90257c1d6a91cee720151b69e0c2020cf6: xen-netback: stop vif thread
spinning if frontend is unresponsive) solved the spinning thread problem,
however caused an another one. The receive side can stall, if:
- [THREAD] xenvif_rx_action sets rx_queue_stopped to true
- [INTERRUPT] interrupt happens, and sets rx_event to true
- [THREAD] then xenvif_kthread sets rx_event to false
- [THREAD] rx_work_todo doesn't return true anymore
Also, if interrupt sent but there is still no room in the ring, it take quite a
long time until xenvif_rx_action realize it. This patch ditch that two variable,
and rework rx_work_todo. If the thread finds it can't fit more skb's into the
ring, it saves the last slot estimation into rx_last_skb_slots, otherwise it's
kept as 0. Then rx_work_todo will check if:
- there is something to send to the ring (like before)
- there is space for the topmost packet in the queue
I think that's more natural and optimal thing to test than two bool which are
set somewhere else.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/xen-netback/netback.c')
-rw-r--r-- | drivers/net/xen-netback/netback.c | 16 |
1 files changed, 6 insertions, 10 deletions
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 6b62c3eb8e18..e5284bca2d90 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c | |||
@@ -476,7 +476,6 @@ static void xenvif_rx_action(struct xenvif *vif) | |||
476 | unsigned long offset; | 476 | unsigned long offset; |
477 | struct skb_cb_overlay *sco; | 477 | struct skb_cb_overlay *sco; |
478 | bool need_to_notify = false; | 478 | bool need_to_notify = false; |
479 | bool ring_full = false; | ||
480 | 479 | ||
481 | struct netrx_pending_operations npo = { | 480 | struct netrx_pending_operations npo = { |
482 | .copy = vif->grant_copy_op, | 481 | .copy = vif->grant_copy_op, |
@@ -486,7 +485,7 @@ static void xenvif_rx_action(struct xenvif *vif) | |||
486 | skb_queue_head_init(&rxq); | 485 | skb_queue_head_init(&rxq); |
487 | 486 | ||
488 | while ((skb = skb_dequeue(&vif->rx_queue)) != NULL) { | 487 | while ((skb = skb_dequeue(&vif->rx_queue)) != NULL) { |
489 | int max_slots_needed; | 488 | RING_IDX max_slots_needed; |
490 | int i; | 489 | int i; |
491 | 490 | ||
492 | /* We need a cheap worse case estimate for the number of | 491 | /* We need a cheap worse case estimate for the number of |
@@ -509,9 +508,10 @@ static void xenvif_rx_action(struct xenvif *vif) | |||
509 | if (!xenvif_rx_ring_slots_available(vif, max_slots_needed)) { | 508 | if (!xenvif_rx_ring_slots_available(vif, max_slots_needed)) { |
510 | skb_queue_head(&vif->rx_queue, skb); | 509 | skb_queue_head(&vif->rx_queue, skb); |
511 | need_to_notify = true; | 510 | need_to_notify = true; |
512 | ring_full = true; | 511 | vif->rx_last_skb_slots = max_slots_needed; |
513 | break; | 512 | break; |
514 | } | 513 | } else |
514 | vif->rx_last_skb_slots = 0; | ||
515 | 515 | ||
516 | sco = (struct skb_cb_overlay *)skb->cb; | 516 | sco = (struct skb_cb_overlay *)skb->cb; |
517 | sco->meta_slots_used = xenvif_gop_skb(skb, &npo); | 517 | sco->meta_slots_used = xenvif_gop_skb(skb, &npo); |
@@ -522,8 +522,6 @@ static void xenvif_rx_action(struct xenvif *vif) | |||
522 | 522 | ||
523 | BUG_ON(npo.meta_prod > ARRAY_SIZE(vif->meta)); | 523 | BUG_ON(npo.meta_prod > ARRAY_SIZE(vif->meta)); |
524 | 524 | ||
525 | vif->rx_queue_stopped = !npo.copy_prod && ring_full; | ||
526 | |||
527 | if (!npo.copy_prod) | 525 | if (!npo.copy_prod) |
528 | goto done; | 526 | goto done; |
529 | 527 | ||
@@ -1473,8 +1471,8 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif *vif, | |||
1473 | 1471 | ||
1474 | static inline int rx_work_todo(struct xenvif *vif) | 1472 | static inline int rx_work_todo(struct xenvif *vif) |
1475 | { | 1473 | { |
1476 | return (!skb_queue_empty(&vif->rx_queue) && !vif->rx_queue_stopped) || | 1474 | return !skb_queue_empty(&vif->rx_queue) && |
1477 | vif->rx_event; | 1475 | xenvif_rx_ring_slots_available(vif, vif->rx_last_skb_slots); |
1478 | } | 1476 | } |
1479 | 1477 | ||
1480 | static inline int tx_work_todo(struct xenvif *vif) | 1478 | static inline int tx_work_todo(struct xenvif *vif) |
@@ -1560,8 +1558,6 @@ int xenvif_kthread(void *data) | |||
1560 | if (!skb_queue_empty(&vif->rx_queue)) | 1558 | if (!skb_queue_empty(&vif->rx_queue)) |
1561 | xenvif_rx_action(vif); | 1559 | xenvif_rx_action(vif); |
1562 | 1560 | ||
1563 | vif->rx_event = false; | ||
1564 | |||
1565 | if (skb_queue_empty(&vif->rx_queue) && | 1561 | if (skb_queue_empty(&vif->rx_queue) && |
1566 | netif_queue_stopped(vif->dev)) | 1562 | netif_queue_stopped(vif->dev)) |
1567 | xenvif_start_queue(vif); | 1563 | xenvif_start_queue(vif); |