[Xen-devel] [PATCH] netback accel locking bug fix

[Kieran Mansley <kmansley@xxxxxxxxxxxxxx>]

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