xen-blkback: hold write vbd-lock while swapping the vbd

All paths holding a vbd handle or dereferencing it hold the read vbd-lock.
Swapping the device takes a write-trylock on the vbd. So, we fail a swap
if, for instance, there is an active IO operation.

Orabug: 28651655

Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Bhavesh Davda <bhavesh.davda@oracle.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Brian Maly <brian.maly@oracle.com>

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index ab4ebac4b709df214804f52f3a76ad4d97068bfc..28ecd79ba157274dfe6f97009dd5adcc90986ca1 100644 (file)
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -42,6 +42,7 @@
 #include <linux/delay.h>
 #include <linux/freezer.h>
 #include <linux/bitmap.h>
+#include <linux/rwsem.h>
 
 #include <xen/events.h>
 #include <xen/page.h>
@@ -531,6 +532,10 @@ static int xen_vbd_translate(struct phys_req *req, struct xen_blkif *blkif,
                        goto out;
        }
 
+       /*
+        * vbd_translate is always called with vbd_lock held so we can
+        * safely dereference it
+        */
        req->dev  = vbd->pdevice;
        req->bdev = vbd->bdev;
        rc = 0;
@@ -541,11 +546,19 @@ static int xen_vbd_translate(struct phys_req *req, struct xen_blkif *blkif,
 
 static void xen_vbd_resize(struct xen_blkif *blkif)
 {
-       struct xen_vbd *vbd = &blkif->vbd;
+       struct xen_vbd *vbd;
        struct xenbus_transaction xbt;
        int err;
        struct xenbus_device *dev = xen_blkbk_xenbus(blkif->be);
-       unsigned long long new_size = vbd_sz(vbd);
+       unsigned long long new_size;
+
+       down_read(&blkif->vbd_lock);
+       vbd = &blkif->vbd;
+       new_size = vbd_sz(vbd);
+       if (likely(vbd->size == new_size)) {
+               up_read(&blkif->vbd_lock);
+               return;
+       }
 
        pr_info("VBD Resize: Domid: %d, Device: (%d, %d)\n",
                blkif->domid, MAJOR(vbd->pdevice), MINOR(vbd->pdevice));
@@ -555,6 +568,7 @@ again:
        err = xenbus_transaction_start(&xbt);
        if (err) {
                pr_warn("Error starting transaction\n");
+               up_read(&blkif->vbd_lock);
                return;
        }
        err = xenbus_printf(xbt, dev->nodename, "sectors", "%llu",
@@ -579,9 +593,11 @@ again:
                goto again;
        if (err)
                pr_warn("Error ending transaction\n");
+       up_read(&blkif->vbd_lock);
        return;
 abort:
        xenbus_transaction_end(xbt, 1);
+       up_read(&blkif->vbd_lock);
 }
 
 /*
@@ -629,7 +645,6 @@ int xen_blkif_schedule(void *arg)
 {
        struct xen_blkif_ring *ring = arg;
        struct xen_blkif *blkif = ring->blkif;
-       struct xen_vbd *vbd = &blkif->vbd;
        unsigned long timeout;
        int ret;
 
@@ -637,8 +652,7 @@ int xen_blkif_schedule(void *arg)
        while (!kthread_should_stop()) {
                if (try_to_freeze())
                        continue;
-               if (unlikely(vbd->size != vbd_sz(vbd)))
-                       xen_vbd_resize(blkif);
+               xen_vbd_resize(blkif);
 
                timeout = msecs_to_jiffies(LRU_INTERVAL);
 
@@ -1234,6 +1248,12 @@ __do_block_io_op(struct xen_blkif_ring *ring)
 
                barrier();
 
+               /*
+                * To ensure that this vbd hangs around, we hold onto the lock
+                * until completion
+                */
+               down_read(&ring->blkif->vbd_lock);
+
                if (likely(valid_req)) {
                        switch (req.operation) {
                        case BLKIF_OP_READ:
@@ -1480,6 +1500,8 @@ static void make_response(struct xen_blkif_ring *ring, u64 id,
        union blkif_back_rings *blk_rings;
        int notify;
 
+       up_read(&ring->blkif->vbd_lock);
+
        spin_lock_irqsave(&ring->blk_ring_lock, flags);
        blk_rings = &ring->blk_rings;
        /* Place on the response ring for the relevant domain. */

diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 921b084f35d822b0d8b3e12a1b553572e30664bd..242808a24be085dcf98f68995b17f2a4f9823f60 100644 (file)
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -331,6 +331,21 @@ struct xen_blkif {
        unsigned int            nr_rings;
        unsigned int            feature_gnt_persistent:1;
        unsigned int            overflow_max_grants:1;
+       struct rw_semaphore     vbd_lock;
+};
+
+struct blkif_params {
+       unsigned int major;
+       unsigned int minor;
+       unsigned int readonly;
+       unsigned int cdrom;
+};
+
+struct backend_info {
+       struct xenbus_device    *dev;
+       struct xen_blkif        *blkif;
+       struct xenbus_watch     backend_watch;
+       struct blkif_params     params;
 };
 
 struct seg_buf {

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 50ca4f591d525091b3ed37bb575821518e1364ff..16cf1381508ec0b29e2710a2cda6dc28d787cb8d 100644 (file)
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -19,6 +19,7 @@
 #include <stdarg.h>
 #include <linux/module.h>
 #include <linux/kthread.h>
+#include <linux/rwsem.h>
 #include <xen/events.h>
 #include <xen/grant_table.h>
 #include "common.h"
@@ -26,20 +27,6 @@
 /* On the XenBus the max length of 'ring-ref%u'. */
 #define RINGREF_NAME_LEN (20)
 
-struct blkif_params {
-       unsigned int major;
-       unsigned int minor;
-       unsigned int readonly;
-       unsigned int cdrom;
-};
-
-struct backend_info {
-       struct xenbus_device    *dev;
-       struct xen_blkif        *blkif;
-       struct xenbus_watch     backend_watch;
-       struct blkif_params     params;
-};
-
 static struct kmem_cache *xen_blkif_cachep;
 static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);
@@ -195,6 +182,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
        atomic_set(&blkif->refcnt, 1);
        init_completion(&blkif->drain_complete);
        INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
+       init_rwsem(&blkif->vbd_lock);
 
        return blkif;
 }
@@ -790,6 +778,7 @@ static void backend_changed(struct xenbus_watch *watch,
        struct blkif_params oldp, newp;
        struct xen_vbd vbd;
        bool swap = false;
+       int locked = 0;
 
        pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
 
@@ -799,9 +788,17 @@ static void backend_changed(struct xenbus_watch *watch,
 
        if (be->params.major | be->params.minor) {
                err = validate_swap_params(be, &newp);
-               if (err)
+
+               if (!err) {
+                       locked = down_write_trylock(&be->blkif->vbd_lock);
+                       if (locked)
+                               swap = true;
+                       else
+                               err = -EAGAIN;
+               }
+
+               if (err) /* !valid || !locked */
                        goto out;
-               swap = true;
        }
 
        err = xen_vbd_create(be->blkif, handle, &newp, &vbd);
@@ -818,6 +815,12 @@ static void backend_changed(struct xenbus_watch *watch,
        be->params = newp;
 
        if (swap) {
+               if (be->blkif->vbd.bdev == NULL) {
+                       /* Should not happen; Bail out */
+                       WARN(1, "xen-blkback: invalid blkif->vbd.bdev\n");
+                       goto flush_sysfs_out;
+               }
+
                /* Flush, invalidate extant mappings */
                err = flush_bdev_mapping(dev, &be->blkif->vbd);
                if (err) {
@@ -871,6 +874,8 @@ out:
                xenbus_dev_fatal(dev, err,
                        "Failed in writing oracle/active-physical-device");
        }
+       if (locked)
+               up_write(&be->blkif->vbd_lock);
 }
 
 /*
@@ -976,11 +981,14 @@ static void connect(struct backend_info *be)
 
        pr_debug("%s %s\n", __func__, dev->otherend);
 
+       down_read(&be->blkif->vbd_lock);
+
        /* Supply the information about the device the frontend needs */
 again:
        err = xenbus_transaction_start(&xbt);
        if (err) {
                xenbus_dev_fatal(dev, err, "starting transaction");
+               up_read(&be->blkif->vbd_lock);
                return;
        }
 
@@ -1040,9 +1048,11 @@ again:
                xenbus_dev_fatal(dev, err, "%s: switching to Connected state",
                                 dev->nodename);
 
+       up_read(&be->blkif->vbd_lock);
        return;
  abort:
        xenbus_transaction_end(xbt, 1);
+       up_read(&be->blkif->vbd_lock);
 }
 
 void xen_blkbk_free_req(struct pending_req *req)