Re: [Xen-devel] [PATCH v3 0/3] map grant refs at pfn = mfn

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 07, 2014 at 08:59:30AM +0100, Thomas Leonard wrote:
[...]
> >
> > If you do "ps aux | grep vif" do you see some kthreads? If so, what is
> > the status of these threads?
> 
> root@cubietruck:~# xl list
> Name                                        ID   Mem VCPUs      State   
> Time(s)
> Domain-0                                     0   512     2     r-----      
> 22.0
> (null)                                       1     0     1     --ps-d       
> 0.1
> 
> root@cubietruck:~# ps aux | grep vif
> root      1189  0.0  0.0      0     0 ?        S    07:50   0:00
> [vif1.0-q0-guest]
> root      1190  0.0  0.0      0     0 ?        S    07:50   0:00
> [vif1.0-q0-deall]
> root      1242  0.0  0.2   3388  1052 pts/0    S+   07:51   0:00 grep
> --color=auto vif
> 
> > What's the status of "xenwatch" thread?
> 
> root@cubietruck:~# ps aux | grep xenwatch
> root        18  0.0  0.0      0     0 ?        D    07:49   0:00 [xenwatch]
> 
> root@cubietruck:~# cat /proc/18/wchan
> xenvif_disconnect
> 

OK, so the ref-count is messed up. My patch is buggy.

Another simplier fix came to mind this morning. Can you give it a go?
I've run preliminary test on x86, this patch at least doesn't cause
regression for me.

Wei.

---8<---
From 9b7db1c85798a0f25ce845815d20ff331aaf15c1 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Thu, 7 Aug 2014 11:33:37 +0100
Subject: [PATCH] xen-netback: don't stop dealloc kthread until all packets
 are processed

Move xenvif_wait_unmap_timeout before stopping dealloc thread, so that
we guarantee all inflight packets are eventually unmapped.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 drivers/net/xen-netback/interface.c |   30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index 5f11d1d..ce871db 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -655,6 +655,20 @@ void xenvif_disconnect(struct xenvif *vif)
        struct xenvif_queue *queue = NULL;
        unsigned int num_queues = vif->num_queues;
        unsigned int queue_index;
+       /* Here we want to avoid timeout messages if an skb can be legitimately
+        * stuck somewhere else. Realistically this could be an another vif's
+        * internal or QDisc queue. That another vif also has this
+        * rx_drain_timeout_msecs timeout, but the timer only ditches the
+        * internal queue. After that, the QDisc queue can put in worst case
+        * XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS skbs into that another vif's
+        * internal queue, so we need several rounds of such timeouts until we
+        * can be sure that no another vif should have skb's from us. We are
+        * not sending more skb's, so newly stuck packets are not interesting
+        * for us here.
+        */
+       unsigned int worst_case_skb_lifetime = (rx_drain_timeout_msecs/1000) *
+               DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / 
MAX_SKB_FRAGS));
+
 
        if (netif_carrier_ok(vif->dev))
                xenvif_carrier_off(vif);
@@ -673,6 +687,8 @@ void xenvif_disconnect(struct xenvif *vif)
                        queue->task = NULL;
                }
 
+               xenvif_wait_unmap_timeout(queue, worst_case_skb_lifetime);
+
                if (queue->dealloc_task) {
                        kthread_stop(queue->dealloc_task);
                        queue->dealloc_task = NULL;
@@ -706,25 +722,11 @@ void xenvif_free(struct xenvif *vif)
        struct xenvif_queue *queue = NULL;
        unsigned int num_queues = vif->num_queues;
        unsigned int queue_index;
-       /* Here we want to avoid timeout messages if an skb can be legitimately
-        * stuck somewhere else. Realistically this could be an another vif's
-        * internal or QDisc queue. That another vif also has this
-        * rx_drain_timeout_msecs timeout, but the timer only ditches the
-        * internal queue. After that, the QDisc queue can put in worst case
-        * XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS skbs into that another vif's
-        * internal queue, so we need several rounds of such timeouts until we
-        * can be sure that no another vif should have skb's from us. We are
-        * not sending more skb's, so newly stuck packets are not interesting
-        * for us here.
-        */
-       unsigned int worst_case_skb_lifetime = (rx_drain_timeout_msecs/1000) *
-               DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / 
MAX_SKB_FRAGS));
 
        unregister_netdev(vif->dev);
 
        for (queue_index = 0; queue_index < num_queues; ++queue_index) {
                queue = &vif->queues[queue_index];
-               xenvif_wait_unmap_timeout(queue, worst_case_skb_lifetime);
                xenvif_deinit_queue(queue);
        }
 
-- 
1.7.10.4

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel