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 -v3 3/3] xen-blkback: refactor vbd remove/disconnect.

To: Joe Jin <joe.jin@xxxxxxxxxx>
Subject: [Xen-devel] [PATCH -v3 3/3] xen-blkback: refactor vbd remove/disconnect.
From: Joe Jin <joe.jin@xxxxxxxxxx>
Date: Thu, 04 Aug 2011 15:25:44 +0800
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Jens Axboe <jaxboe@xxxxxxxxxxxx>, Greg Marsden <greg.marsden@xxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Annie Li <annie.li@xxxxxxxxxx>, Kurt C Hackel <KURT.HACKEL@xxxxxxxxxx>, Daniel Stodden <daniel.stodden@xxxxxxxxxx>
Delivery-date: Thu, 04 Aug 2011 00:27:04 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E3A486D.7060506@xxxxxxxxxx>
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: <4E3A486D.7060506@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110707 Thunderbird/5.0
This patch refactor vbd remove/disconnect.
1. Add blkback shutdown watch for the remove/disconnect.
2. Don't disconnect backend when frontend state is XenbusStateClosing
   until frontend state changed to XenbusStateClosed.

Signed-off-by: Joe Jin <joe.jin@xxxxxxxxxx>
Cc: Daniel Stodden <daniel.stodden@xxxxxxxxxx>
Cc: Jens Axboe <jaxboe@xxxxxxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Annie Li <annie.li@xxxxxxxxxx>
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
---
 drivers/block/xen-blkback/xenbus.c |  206 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 185 insertions(+), 21 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c 
b/drivers/block/xen-blkback/xenbus.c
index 3f129b4..1990d1c 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -25,16 +25,25 @@ struct backend_info {
        struct xenbus_device    *dev;
        struct xen_blkif        *blkif;
        struct xenbus_watch     backend_watch;
+       struct xenbus_watch     shutdown_watch;
        unsigned                major;
        unsigned                minor;
        char                    *mode;
+       bool                    group_added;
+       char                    *nodename;
+       atomic_t                refcnt;
+       pid_t                   kthread_pid;
+       bool                    shutdown_signalled;
 };
 
+DEFINE_SEMAPHORE(blkback_dev_sem);
+
 static struct kmem_cache *xen_blkif_cachep;
 static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);
 static void backend_changed(struct xenbus_watch *, const char **,
                            unsigned int);
+static void xen_vbd_free(struct xen_vbd *vbd);
 
 struct xenbus_device *xen_blkbk_xenbus(struct backend_info *be)
 {
@@ -99,6 +108,16 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
                blkif->xenblkd = NULL;
                xenbus_dev_error(blkif->be->dev, err, "start xenblkd");
        }
+
+       blkif->be->kthread_pid = blkif->xenblkd->pid;
+       atomic_inc(&blkif->be->refcnt);
+
+       err = xenbus_printf(XBT_NIL, blkif->be->dev->nodename, "kthread-pid",
+                           "%d", blkif->xenblkd->pid);
+       if (err) {
+               xenbus_dev_error(blkif->be->dev, err, "write kthread-pid");
+               return;
+       }
 }
 
 static struct xen_blkif *xen_blkif_alloc(domid_t domid)
@@ -213,11 +232,6 @@ static int xen_blkif_map(struct xen_blkif *blkif, unsigned 
long shared_page,
 
 static void xen_blkif_disconnect(struct xen_blkif *blkif)
 {
-       if (blkif->xenblkd) {
-               kthread_stop(blkif->xenblkd);
-               blkif->xenblkd = NULL;
-       }
-
        atomic_dec(&blkif->refcnt);
        wait_event(blkif->waiting_to_free, atomic_read(&blkif->refcnt) == 0);
        atomic_inc(&blkif->refcnt);
@@ -296,6 +310,7 @@ VBD_SHOW(mode, "%s\n", be->mode);
 int xenvbd_sysfs_addif(struct xenbus_device *dev)
 {
        int error;
+       struct backend_info *be = dev_get_drvdata(&dev->dev);
 
        error = device_create_file(&dev->dev, &dev_attr_physical_device);
        if (error)
@@ -309,6 +324,8 @@ int xenvbd_sysfs_addif(struct xenbus_device *dev)
        if (error)
                goto fail3;
 
+       be->group_added = true;
+
        return 0;
 
 fail3: sysfs_remove_group(&dev->dev.kobj, &xen_vbdstat_group);
@@ -319,11 +336,73 @@ fail1:    device_remove_file(&dev->dev, 
&dev_attr_physical_device);
 
 void xenvbd_sysfs_delif(struct xenbus_device *dev)
 {
+       struct backend_info *be = dev_get_drvdata(&dev->dev);
+       if (!be->group_added)
+               return;
        sysfs_remove_group(&dev->dev.kobj, &xen_vbdstat_group);
        device_remove_file(&dev->dev, &dev_attr_mode);
        device_remove_file(&dev->dev, &dev_attr_physical_device);
+       be->group_added = false;
+}
+
+static int xenvbd_kthread_remove(struct backend_info *be)
+{
+       struct xen_blkif *blkif = be->blkif;
+
+       if (!blkif || !blkif->xenblkd)
+               return 0;
+
+       blkif->remove_requested = true;
+       wake_up_process(blkif->xenblkd);
+
+       return -EBUSY;
 }
 
+static void xenvbd_signal_shutdown(struct backend_info *be)
+{
+       int err;
+
+       down(&blkback_dev_sem);
+
+       if (be->shutdown_signalled)
+               goto out;
+
+       err = xenbus_write(XBT_NIL, be->nodename, "shutdown-done", "");
+       if (err)
+               WPRINTK("Error writing shutdown-done for %s: %d\n",
+                       be->nodename, err);
+
+       if (be->dev)
+               xenbus_switch_state(be->dev, XenbusStateClosed);
+
+       be->shutdown_signalled = true;
+
+ out:
+       up(&blkback_dev_sem);
+}
+
+static void backend_release(struct backend_info *be)
+{
+       struct xen_blkif *blkif = be->blkif;
+
+       if (current->pid == be->kthread_pid)
+               xenvbd_signal_shutdown(be);
+
+       if (!atomic_dec_and_test(&be->refcnt))
+               return;
+
+       xenvbd_signal_shutdown(be);
+
+       if (blkif) {
+               xen_blkif_disconnect(blkif);
+               xen_vbd_free(&blkif->vbd);
+               xen_blkif_free(blkif);
+               be->blkif = NULL;
+       }
+
+       kfree(be->nodename);
+       kfree(be);
+}
 
 static void xen_vbd_free(struct xen_vbd *vbd)
 {
@@ -332,6 +411,12 @@ static void xen_vbd_free(struct xen_vbd *vbd)
        vbd->bdev = NULL;
 }
 
+void xen_vbd_sync(struct xen_vbd *vbd)
+{
+       if (vbd->bdev)
+               fsync_bdev(vbd->bdev);
+}
+
 static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
                          unsigned major, unsigned minor, int readonly,
                          int cdrom)
@@ -384,8 +469,9 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
 
        DPRINTK("");
 
-       if (be->major || be->minor)
-               xenvbd_sysfs_delif(dev);
+       down(&blkback_dev_sem);
+       be->dev = NULL;
+       up(&blkback_dev_sem);
 
        if (be->backend_watch.node) {
                unregister_xenbus_watch(&be->backend_watch);
@@ -393,18 +479,76 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
                be->backend_watch.node = NULL;
        }
 
-       if (be->blkif) {
-               xen_blkif_disconnect(be->blkif);
-               xen_vbd_free(&be->blkif->vbd);
-               xen_blkif_free(be->blkif);
-               be->blkif = NULL;
+       if (be->shutdown_watch.node) {
+               unregister_xenbus_watch(&be->shutdown_watch);
+               kfree(be->shutdown_watch.node);
+               be->shutdown_watch.node = NULL;
        }
 
-       kfree(be);
+       if (xenvbd_kthread_remove(be))
+               WPRINTK("BAD REMOVE REQUEST for %s\n", be->nodename);
+
+       xenvbd_sysfs_delif(dev);
+       backend_release(be);
+
        dev_set_drvdata(&dev->dev, NULL);
+
        return 0;
 }
 
+/*
+ * called by kthread when closing
+ */
+void xen_blkback_close(struct xen_blkif *blkif)
+{
+       struct xenbus_device *dev = blkif->be->dev;
+
+       xen_blkif_disconnect(blkif);
+       xen_vbd_sync(&blkif->vbd);
+       blkif->remove_requested = false;
+
+       down(&blkback_dev_sem);
+       if (blkif->be->dev)
+               xenvbd_sysfs_delif(blkif->be->dev);
+       up(&blkback_dev_sem);
+
+       backend_release(blkif->be);
+       blkif->xenblkd = NULL;
+       device_unregister(&dev->dev);
+}
+
+static void xenvbd_start_shutdown(struct xenbus_watch *watch,
+                          const char **vec, unsigned int length)
+{
+       int err;
+       char *type;
+       unsigned int len;
+       struct backend_info *be
+               = container_of(watch, struct backend_info, shutdown_watch);
+       struct xenbus_device *dev = be->dev;
+
+       if (be->shutdown_signalled)
+               return;
+
+       type = xenbus_read(XBT_NIL, dev->nodename, "shutdown-request", &len);
+       err  = (IS_ERR(type) ? PTR_ERR(type) : 0);
+
+       if (XENBUS_EXIST_ERR(err))
+               return;
+
+       if (err) {
+               xenbus_dev_fatal(dev, err, "reading shutdown-request");
+               return;
+       }
+
+       xenbus_switch_state(dev, XenbusStateClosing);
+
+       if (len == sizeof("force") - 1 && !memcmp(type, "force", len))
+               if (!xenvbd_kthread_remove(be))
+                       xenvbd_signal_shutdown(be); /* shutdown immediately */
+
+       kfree(type);
+}
 int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
                              struct backend_info *be, int state)
 {
@@ -437,6 +581,15 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
        }
        be->dev = dev;
        dev_set_drvdata(&dev->dev, be);
+       atomic_set(&be->refcnt, 1);
+
+       be->nodename = kasprintf(GFP_KERNEL, "%s", dev->nodename);
+       if (!be->nodename) {
+               xenbus_dev_fatal(dev, -ENOMEM,
+                                "allocating backend structure");
+               kfree(be);
+               return -ENOMEM;
+       }
 
        be->blkif = xen_blkif_alloc(dev->otherend_id);
        if (IS_ERR(be->blkif)) {
@@ -454,6 +607,12 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
        if (err)
                goto fail;
 
+       err = xenbus_watch_pathfmt(dev, &be->shutdown_watch,
+                                  xenvbd_start_shutdown, "%s/%s",
+                                  dev->nodename, "shutdown-request");
+       if (err)
+               goto fail;
+
        err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err)
                goto fail;
@@ -567,13 +726,17 @@ static void frontend_changed(struct xenbus_device *dev,
        struct backend_info *be = dev_get_drvdata(&dev->dev);
        int err;
 
-       DPRINTK("%s", xenbus_strstate(frontend_state));
+       DPRINTK("%s: %s", dev->nodename, xenbus_strstate(frontend_state));
 
        switch (frontend_state) {
        case XenbusStateInitialising:
                if (dev->state == XenbusStateClosed) {
                        pr_info(DRV_PFX "%s: prepare for reconnect\n",
                                dev->nodename);
+
+                       xenbus_rm(XBT_NIL, dev->nodename, "shutdown-done");
+                       be->shutdown_signalled = false;
+
                        xenbus_switch_state(dev, XenbusStateInitWait);
                }
                break;
@@ -590,7 +753,7 @@ static void frontend_changed(struct xenbus_device *dev,
 
                /*
                 * Enforce precondition before potential leak point.
-                * blkif_disconnect() is idempotent.
+                * xen_blkif_disconnect() is idempotent.
                 */
                xen_blkif_disconnect(be->blkif);
 
@@ -601,17 +764,16 @@ static void frontend_changed(struct xenbus_device *dev,
                break;
 
        case XenbusStateClosing:
-               xen_blkif_disconnect(be->blkif);
                xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
-               xenbus_switch_state(dev, XenbusStateClosed);
-               if (xenbus_dev_is_online(dev))
-                       break;
-               /* fall through if not online */
+               if (!xenvbd_kthread_remove(be))
+                       xenvbd_signal_shutdown(be);
+               break;
+
        case XenbusStateUnknown:
-               /* implies blkif_disconnect() via blkback_remove() */
+               /* implies xen_blkif_disconnect() via blkback_remove() */
                device_unregister(&dev->dev);
                break;
 
@@ -620,6 +782,8 @@ static void frontend_changed(struct xenbus_device *dev,
                                 frontend_state);
                break;
        }
+
+       DPRINTK("%s: %s", dev->nodename, xenbus_strstate(dev->state));
 }
 
 


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