WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH 10/22] Add a fall-back poller, in case finish message

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH 10/22] Add a fall-back poller, in case finish messages get stuck somewhere.
From: <steven.smith@xxxxxxxxxx>
Date: Sun, 4 Oct 2009 16:04:03 +0100
Cc: keir.fraser@xxxxxxxxxx, Steven Smith <steven.smith@xxxxxxxxxx>, jean.guyader@xxxxxxxxxx
Delivery-date: Sun, 04 Oct 2009 08:30:22 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <cover.1254666837.git.ssmith@xxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <cover.1254666837.git.ssmith@xxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
We try to avoid the event channel notification when sending finish
messages, for performance reasons, but that can lead to a deadlock if
you have a lot of packets going in one direction and nothing coming
the other way.  Fix it by just polling for messages every second when
there are unfinished packets outstanding.

Signed-off-by: Steven Smith <steven.smith@xxxxxxxxxx>
---
 drivers/xen/netchannel2/Makefile           |    2 +-
 drivers/xen/netchannel2/chan.c             |    5 ++-
 drivers/xen/netchannel2/netchannel2_core.h |   10 +++++
 drivers/xen/netchannel2/poll.c             |   59 ++++++++++++++++++++++++++++
 drivers/xen/netchannel2/xmit_packet.c      |    3 +
 5 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 drivers/xen/netchannel2/poll.c

diff --git a/drivers/xen/netchannel2/Makefile b/drivers/xen/netchannel2/Makefile
index bdad6da..d6641a1 100644
--- a/drivers/xen/netchannel2/Makefile
+++ b/drivers/xen/netchannel2/Makefile
@@ -1,7 +1,7 @@
 obj-$(CONFIG_XEN_NETCHANNEL2) += netchannel2.o
 
 netchannel2-objs := chan.o netchan2.o rscb.o util.o \
-       xmit_packet.o recv_packet.o
+       xmit_packet.o recv_packet.o poll.o
 
 ifeq ($(CONFIG_XEN_NETDEV2_BACKEND),y)
 netchannel2-objs += netback2.o
diff --git a/drivers/xen/netchannel2/chan.c b/drivers/xen/netchannel2/chan.c
index e3ad981..a4b83a1 100644
--- a/drivers/xen/netchannel2/chan.c
+++ b/drivers/xen/netchannel2/chan.c
@@ -23,6 +23,7 @@ static irqreturn_t nc2_int(int irq, void *dev_id)
 
        if (ncr->irq == -1)
                return IRQ_HANDLED;
+       ncr->last_event = jiffies;
        if (ncr->cons_ring.sring->prod != ncr->cons_ring.cons_pvt ||
            ncr->interface->is_stopped)
                nc2_kick(ncr);
@@ -293,6 +294,8 @@ int init_ring_pair(struct netchannel2_ring_pair *ncrp,
                                          &ncrp->gref_pool) < 0)
                return -1;
 
+       nc2_init_poller(ncrp);
+
        netif_napi_add(ncrp->interface->net_device, &ncrp->napi,
                       process_ring, 64);
        napi_enable(&ncrp->napi);
@@ -509,7 +512,7 @@ static void nc2_detach_ring(struct netchannel2_ring_pair 
*ncrp)
 {
        if (!ncrp->is_attached)
                return;
-
+       nc2_stop_polling(ncrp);
        napi_disable(&ncrp->napi);
        _detach_rings(ncrp);
 }
diff --git a/drivers/xen/netchannel2/netchannel2_core.h 
b/drivers/xen/netchannel2/netchannel2_core.h
index 6ae273d..7304451 100644
--- a/drivers/xen/netchannel2/netchannel2_core.h
+++ b/drivers/xen/netchannel2/netchannel2_core.h
@@ -130,6 +130,11 @@ struct netchannel2_ring_pair {
 
        struct napi_struct napi;
 
+       /* jiffies the last time the interrupt fired.  Not
+          synchronised at all, because it doesn't usually matter if
+          it's a bit off. */
+       unsigned last_event;
+
        /* Protected by the lock.  Initialised at attach_ring() time
           and de-initialised at detach_ring() time. */
        struct netchannel2_prod_ring prod_ring;
@@ -139,6 +144,7 @@ struct netchannel2_ring_pair {
 
        unsigned max_count_frags_no_event;
        unsigned expected_finish_messages;
+       struct timer_list polling_timer;
 
        domid_t otherend_id;
 
@@ -348,4 +354,8 @@ void receive_pending_skbs(struct sk_buff_head *rx_queue);
 void nc2_queue_purge(struct netchannel2_ring_pair *ncrp,
                     struct sk_buff_head *queue);
 
+void nc2_init_poller(struct netchannel2_ring_pair *ncrp);
+void nc2_start_polling(struct netchannel2_ring_pair *ncrp);
+void nc2_stop_polling(struct netchannel2_ring_pair *ncrp);
+
 #endif /* !NETCHANNEL2_CORE_H__ */
diff --git a/drivers/xen/netchannel2/poll.c b/drivers/xen/netchannel2/poll.c
new file mode 100644
index 0000000..42ca0d5
--- /dev/null
+++ b/drivers/xen/netchannel2/poll.c
@@ -0,0 +1,59 @@
+/* There are a couple of places where we try to minimise wakeups in
+   ways which work in the vast majority of cases, but occasionally
+   cause a needed event to be lost.  Compensate for those with a 1Hz
+   ticker.  The ticker runs whenever we have outstanding TX packets.
+   Once it's running, we never try to modify it, and instead just let
+   it run out. */
+/* If we're relying on this timer for correctness then performance is
+   going to be absolutely dire, but it should be sufficient to avoid
+   outright deadlocks. */
+#include <linux/kernel.h>
+#include <linux/timer.h>
+#include "netchannel2_core.h"
+
+#define TICKER_INTERVAL (HZ)
+
+static void poll_timer(unsigned long arg)
+{
+       struct netchannel2_ring_pair *ncrp =
+               (struct netchannel2_ring_pair *)arg;
+
+       /* If the ring appears to be behaving ``normally'', increase
+          the number of messages which we're allowed to have
+          outstanding by some small amount.  If it looks like we've
+          deadlocked, halve it. */
+       /* Arbitrarily define ``normal'' to be at least one interrupt
+          every 100ms, and a small amount to be 10. */
+       /* We don't synchronise against concurrent readers of
+          max_count_frags_no_event, because it doesn't matter too
+          much if it's slightly wrong.  We don't need to worry about
+          concurrent writers, because this timer is the only thing
+          which can change it, and it's only ever run on one cpu at a
+          time. */
+       if (jiffies - ncrp->last_event > HZ/10)
+               ncrp->max_count_frags_no_event /= 2;
+       else if (ncrp->max_count_frags_no_event + 10 <=
+                MAX_MAX_COUNT_FRAGS_NO_EVENT)
+               ncrp->max_count_frags_no_event += 10;
+
+       if (ncrp->expected_finish_messages == 0)
+               return;
+       if (ncrp->cons_ring.sring->prod != ncrp->cons_ring.cons_pvt)
+               nc2_kick(ncrp);
+       nc2_start_polling(ncrp);
+}
+
+void nc2_init_poller(struct netchannel2_ring_pair *ncrp)
+{
+       setup_timer(&ncrp->polling_timer, poll_timer, (unsigned long)ncrp);
+}
+
+void nc2_start_polling(struct netchannel2_ring_pair *ncrp)
+{
+       mod_timer(&ncrp->polling_timer, jiffies + TICKER_INTERVAL);
+}
+
+void nc2_stop_polling(struct netchannel2_ring_pair *ncrp)
+{
+       del_timer_sync(&ncrp->polling_timer);
+}
diff --git a/drivers/xen/netchannel2/xmit_packet.c 
b/drivers/xen/netchannel2/xmit_packet.c
index 92fbabf..a693a75 100644
--- a/drivers/xen/netchannel2/xmit_packet.c
+++ b/drivers/xen/netchannel2/xmit_packet.c
@@ -165,6 +165,9 @@ int nc2_really_start_xmit(struct netchannel2_ring_pair 
*ncrp,
 
        if (skb_co->tp) {
                ncrp->expected_finish_messages++;
+               if (ncrp->expected_finish_messages == 1 &&
+                   !timer_pending(&ncrp->polling_timer))
+                       nc2_start_polling(ncrp);
                /* We're now ready to accept a FINISH message for this
                   packet. */
                skb_co->expecting_finish = 1;
-- 
1.6.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>