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] Re: [PATCH] Fix blk{back,tap} sysfs race

To: "Keir Fraser" <keir.fraser@xxxxxxxxxx>, "Joe Jin" <joe.jin@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] Fix blk{back,tap} sysfs race
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Thu, 21 Jan 2010 08:23:19 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, deepak.patel@xxxxxxxxxx, greg.marsden@xxxxxxxxxx
Delivery-date: Thu, 21 Jan 2010 00:23:36 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20100121004658.GA28792@xxxxxxxxxxxxxxxxxxxxxxx>
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: <20100121004658.GA28792@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
While I would have wanted it to use DEFINE_RWLOCK(), it's probably
okay without. Or maybe you could adjust this as you check it in, Keir?

Acked-by: Jan Beulich <jbeulich@xxxxxxxxxx>

>>> "Joe Jin" <joe.jin@xxxxxxxxxx> 21.01.10 01:46 >>>
Hi, 

Read blk{back,tap} statistics info after remove vbd device(s) kernel will crash.
Below patch will fix the race, please review and apply.

Signed-off-by: Joe Jin <joe.jin@xxxxxxxxxx>
Cc:  Keir Fraser <keir.fraser@xxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxxxx>

---
 blkback/xenbus.c |   19 ++++++++++++++++---
 blktap/xenbus.c  |   18 +++++++++++++++---
 2 files changed, 31 insertions(+), 6 deletions(-)

diff -r 0bec29c94ce9 drivers/xen/blkback/xenbus.c
--- a/drivers/xen/blkback/xenbus.c      Mon Jan 18 14:50:43 2010 +0000
+++ b/drivers/xen/blkback/xenbus.c      Thu Jan 21 08:38:58 2010 +0800
@@ -26,6 +26,8 @@
 #define DPRINTK(fmt, args...)                          \
        pr_debug("blkback/xenbus (%s:%d) " fmt ".\n",   \
                 __FUNCTION__, __LINE__, ##args)
+
+static rwlock_t sysfs_read_lock = RW_LOCK_UNLOCKED;
 
 struct backend_info
 {
@@ -104,10 +106,19 @@
                                   struct device_attribute *attr,       \
                                   char *buf)                           \
        {                                                               \
-               struct xenbus_device *dev = to_xenbus_device(_dev);     \
-               struct backend_info *be = dev->dev.driver_data;         \
+               ssize_t ret = -ENODEV;                                  \
+               struct xenbus_device *dev;                              \
+               struct backend_info *be;                                \
                                                                        \
-               return sprintf(buf, format, ##args);                    \
+               if (!get_device(_dev))                                  \
+                       return ret;                                     \
+               dev = to_xenbus_device(_dev);                           \
+               read_lock(&sysfs_read_lock);                            \
+               if ((be = dev->dev.driver_data) != NULL)                \
+                       ret = sprintf(buf, format, ##args);             \
+               read_unlock(&sysfs_read_lock);                          \
+               put_device(_dev);                                       \
+               return ret;                                             \
        }                                                               \
        static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
 
@@ -173,6 +184,7 @@
 
        DPRINTK("");
 
+       write_lock(&sysfs_read_lock);
        if (be->major || be->minor)
                xenvbd_sysfs_delif(dev);
 
@@ -191,6 +203,7 @@
 
        kfree(be);
        dev->dev.driver_data = NULL;
+       write_unlock(&sysfs_read_lock);
        return 0;
 }
 
diff -r 0bec29c94ce9 drivers/xen/blktap/xenbus.c
--- a/drivers/xen/blktap/xenbus.c       Mon Jan 18 14:50:43 2010 +0000
+++ b/drivers/xen/blktap/xenbus.c       Thu Jan 21 08:38:58 2010 +0800
@@ -50,6 +50,7 @@
        int group_added;
 };
 
+static rwlock_t sysfs_read_lock = RW_LOCK_UNLOCKED;
 
 static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);
@@ -122,10 +123,19 @@
                                   struct device_attribute *attr,       \
                                   char *buf)                           \
        {                                                               \
-               struct xenbus_device *dev = to_xenbus_device(_dev);     \
-               struct backend_info *be = dev->dev.driver_data;         \
+               ssize_t ret = -ENODEV;                                  \
+               struct xenbus_device *dev;                              \
+               struct backend_info *be;                                \
                                                                        \
-               return sprintf(buf, format, ##args);                    \
+               if (!get_device(_dev))                                  \
+                       return ret;                                     \
+               dev = to_xenbus_device(_dev);                           \
+               read_lock(&sysfs_read_lock);                            \
+               if ((be = dev->dev.driver_data) != NULL)                \
+                       ret = sprintf(buf, format, ##args);             \
+               read_unlock(&sysfs_read_lock);                          \
+               put_device(_dev);                                       \
+               return ret;                                             \
        }                                                               \
        static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
 
@@ -170,6 +180,7 @@
 {
        struct backend_info *be = dev->dev.driver_data;
 
+       write_lock(&sysfs_read_lock);
        if (be->group_added)
                xentap_sysfs_delif(be->dev);
        if (be->backend_watch.node) {
@@ -187,6 +198,7 @@
        }
        kfree(be);
        dev->dev.driver_data = NULL;
+       write_unlock(&sysfs_read_lock);
        return 0;
 }
 


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

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