[Xen-changelog] [linux-2.6.18-xen] sfc_netfront: Only clear tx_skb when ready for netif_wake_queue

["Xen patchbot-linux-2.6.18-xen" <patchbot-linux-2.6.18-xen@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1238497253 -3600
# Node ID 3a4410c4504ea64f2c1e873df3234938366050ad
# Parent  ab1d4fbbe4bf93f203e0c4d62c18bd248e4f1d4a
sfc_netfront: Only clear tx_skb when ready for netif_wake_queue
(doing otherwise could result in a lost packet) and document use of
locks to protect tx_skb

Signed-off-by: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
---
 drivers/xen/sfc_netfront/accel.h          |    6 +++--
 drivers/xen/sfc_netfront/accel_netfront.c |    4 +--
 drivers/xen/sfc_netfront/accel_vi.c       |   35 ++++++++++++------------------
 3 files changed, 21 insertions(+), 24 deletions(-)

diff -r ab1d4fbbe4bf -r 3a4410c4504e drivers/xen/sfc_netfront/accel.h
--- a/drivers/xen/sfc_netfront/accel.h  Tue Mar 31 12:00:03 2009 +0100
+++ b/drivers/xen/sfc_netfront/accel.h  Tue Mar 31 12:00:53 2009 +0100
@@ -277,8 +277,10 @@ typedef struct netfront_accel_vnic {
 
        int poll_enabled;
 
-       /** A spare slot for a TX packet.  This is treated as an extension
-        * of the DMA queue. */
+       /** A spare slot for a TX packet.  This is treated as an
+        * extension of the DMA queue.  Reads require either
+        * netfront's tx_lock or the vnic tx_lock; writes require both
+        * locks */
        struct sk_buff *tx_skb;
 
        /** Keep track of fragments of SSR packets */
diff -r ab1d4fbbe4bf -r 3a4410c4504e drivers/xen/sfc_netfront/accel_netfront.c
--- a/drivers/xen/sfc_netfront/accel_netfront.c Tue Mar 31 12:00:03 2009 +0100
+++ b/drivers/xen/sfc_netfront/accel_netfront.c Tue Mar 31 12:00:53 2009 +0100
@@ -65,7 +65,7 @@ static int netfront_accel_netdev_start_x
                BUG_ON(vnic->net_dev != net_dev);
                DPRINTK("%s stopping queue\n", __FUNCTION__);
 
-               /* Netfront's lock protects tx_skb */
+               /* Need netfront's tx_lock and vnic tx_lock to write tx_skb */
                spin_lock_irqsave(&np->tx_lock, flags2);
                BUG_ON(vnic->tx_skb != NULL);
                vnic->tx_skb = skb;
@@ -183,7 +183,7 @@ static int netfront_accel_check_ready(st
 
        BUG_ON(vnic == NULL);
 
-       /* This is protected by netfront's lock */ 
+       /* Read of tx_skb is protected by netfront's tx_lock */ 
        return vnic->tx_skb == NULL;
 }
 
diff -r ab1d4fbbe4bf -r 3a4410c4504e drivers/xen/sfc_netfront/accel_vi.c
--- a/drivers/xen/sfc_netfront/accel_vi.c       Tue Mar 31 12:00:03 2009 +0100
+++ b/drivers/xen/sfc_netfront/accel_vi.c       Tue Mar 31 12:00:53 2009 +0100
@@ -991,39 +991,35 @@ static void netfront_accel_vi_not_busy(n
 {
        struct netfront_info *np = ((struct netfront_info *)
                                    netdev_priv(vnic->net_dev));
-       struct sk_buff *skb;
        int handled;
        unsigned long flags;
-       
+
        /*
-        * TODO if we could safely check tx_skb == NULL and return
-        * early without taking the lock, that would obviously help
-        * performance
-        */
-
-       /* Take the netfront lock which protects tx_skb. */
-       spin_lock_irqsave(&np->tx_lock, flags);
+        * We hold the vnic tx_lock which is sufficient to exclude
+        * writes to tx_skb
+        */
+
        if (vnic->tx_skb != NULL) {
                DPRINTK("%s trying to send spare buffer\n", __FUNCTION__);
                
-               skb = vnic->tx_skb;
-               vnic->tx_skb = NULL;
-
-               spin_unlock_irqrestore(&np->tx_lock, flags);
-
-               handled = netfront_accel_vi_tx_post(vnic, skb);
+               handled = netfront_accel_vi_tx_post(vnic, vnic->tx_skb);
                
-               spin_lock_irqsave(&np->tx_lock, flags);
-
                if (handled != NETFRONT_ACCEL_STATUS_BUSY) {
                        DPRINTK("%s restarting tx\n", __FUNCTION__);
+
+                       /* Need netfront tx_lock and vnic tx_lock to
+                        * write tx_skb */
+                       spin_lock_irqsave(&np->tx_lock, flags);
+
+                       vnic->tx_skb = NULL;
+
                        if (netfront_check_queue_ready(vnic->net_dev)) {
                                netif_wake_queue(vnic->net_dev);
                                NETFRONT_ACCEL_STATS_OP
                                        (vnic->stats.queue_wakes++);
                        }
-               } else {
-                       vnic->tx_skb = skb;
+                       spin_unlock_irqrestore(&np->tx_lock, flags);
+
                }
                
                /*
@@ -1032,7 +1028,6 @@ static void netfront_accel_vi_not_busy(n
                 */
                BUG_ON(handled == NETFRONT_ACCEL_STATUS_CANT);
        }
-       spin_unlock_irqrestore(&np->tx_lock, flags);
 }
 
 

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