From: Ankur Arora Date: Thu, 30 Aug 2018 11:17:49 +0000 (-0700) Subject: xen-blkback: hold write vbd-lock while swapping the vbd X-Git-Tag: v4.1.12-124.31.3~508 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=3470655c33807412ec08e6ad0f5a9f38e822c9ef;p=users%2Fjedix%2Flinux-maple.git 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 Reviewed-by: Darren Kenny Reviewed-by: Bhavesh Davda Reviewed-by: Boris Ostrovsky Signed-off-by: Brian Maly --- diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index ab4ebac4b709..28ecd79ba157 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -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 921b084f35d8..242808a24be0 100644 --- 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 50ca4f591d52..16cf1381508e 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #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)