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] netback accel locking bug fix

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH] netback accel locking bug fix
From: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>
Date: Mon, 05 Nov 2007 14:45:10 +0000
Delivery-date: Mon, 05 Nov 2007 06:45:59 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Please find attached a patch that fixes a bug in the netback
acceleration code.  There was a call to xenbus_read() while a spinlock
was held, and as xenbus_read() can block this was clearly wrong.  The
spinlock is replaced by a mutex.

If this bug fix can weasel its way into 3.2.0 that would be great.

Signed-off-by: Kieran Mansley <kmansley@xxxxxxxxxxxxxx>

Use a mutex for netback accelerators.

diff -r c61274a0a11c drivers/xen/netback/accel.c
--- a/drivers/xen/netback/accel.c       Mon Nov 05 13:02:31 2007 +0000
+++ b/drivers/xen/netback/accel.c       Mon Nov 05 13:48:06 2007 +0000
@@ -33,6 +33,7 @@
 #include <linux/list.h>
 #include <asm/atomic.h>
 #include <xen/xenbus.h>
+#include <linux/mutex.h>
 
 #include "common.h"
 
@@ -48,7 +49,7 @@
  */ 
 static struct list_head accelerators_list;
 /* Lock used to protect access to accelerators_list */
-static spinlock_t accelerators_lock;
+DEFINE_MUTEX(accelerators_mutex);
 
 /* 
  * Compare a backend to an accelerator, and decide if they are
@@ -107,7 +108,7 @@ int netback_connect_accelerator(unsigned
                                struct netback_accel_hooks *hooks)
 {
        struct netback_accelerator *new_accelerator;
-       unsigned eth_name_len, flags;
+       unsigned eth_name_len;
 
        if (version != NETBACK_ACCEL_VERSION) {
                if (version > NETBACK_ACCEL_VERSION) {
@@ -149,9 +150,9 @@ int netback_connect_accelerator(unsigned
 
        atomic_set(&new_accelerator->use_count, 0);
        
-       spin_lock_irqsave(&accelerators_lock, flags);
+       mutex_lock(&accelerators_mutex);
        list_add(&new_accelerator->link, &accelerators_list);
-       spin_unlock_irqrestore(&accelerators_lock, flags);
+       mutex_unlock(&accelerators_mutex);
        
        /* tell existing backends about new plugin */
        xenbus_for_each_backend(new_accelerator, 
@@ -195,12 +196,12 @@ void netback_disconnect_accelerator(int 
        struct netback_accelerator *accelerator, *next;
        unsigned flags;
 
-       spin_lock_irqsave(&accelerators_lock, flags);
+       mutex_lock(&accelerators_mutex);
        list_for_each_entry_safe(accelerator, next, &accelerators_list, link) {
                if (strcmp(eth_name, accelerator->eth_name)) {
                        BUG_ON(atomic_read(&accelerator->use_count) != 0);
                        list_del(&accelerator->link);
-                       spin_unlock_irqrestore(&accelerators_lock, flags);
+                       mutex_unlock(&accelerators_mutex);
 
                        xenbus_for_each_backend(accelerator, 
                                                
netback_accelerator_cleanup_backend);
@@ -210,7 +211,7 @@ void netback_disconnect_accelerator(int 
                        return;
                }
        }
-       spin_unlock_irqrestore(&accelerators_lock, flags);
+       mutex_unlock(&accelerators_mutex);
 }
 EXPORT_SYMBOL_GPL(netback_disconnect_accelerator);
 
@@ -219,13 +220,12 @@ void netback_probe_accelerators(struct b
                                struct xenbus_device *dev)
 {
        struct netback_accelerator *accelerator;
-       unsigned flags;
 
        /* 
         * Check list of accelerators to see if any is suitable, and
         * use it if it is.
         */
-       spin_lock_irqsave(&accelerators_lock, flags);
+       mutex_lock(&accelerators_mutex);
        list_for_each_entry(accelerator, &accelerators_list, link) { 
                if (match_accelerator(dev, be, accelerator) &&
                    try_module_get(accelerator->hooks->owner)) {
@@ -235,7 +235,7 @@ void netback_probe_accelerators(struct b
                        break;
                }
        }
-       spin_unlock_irqrestore(&accelerators_lock, flags);
+       mutex_unlock(&accelerators_mutex);
 }
 
 
@@ -255,5 +255,4 @@ void netif_accel_init(void)
 void netif_accel_init(void)
 {
        INIT_LIST_HEAD(&accelerators_list);
-       spin_lock_init(&accelerators_lock);
-}
+}



Attachment: netback_accel_mutex
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] netback accel locking bug fix, Kieran Mansley <=