From 4dae76705fc8f9854bb732f9944e7ff9ba7a8e9f Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Tue, 13 Mar 2012 18:43:23 -0400 Subject: [PATCH 1/2] xen/blkback: Squash the discard support for 'file' and 'phy' type. The only reason for the distinction was for the special case of 'file' (which is assumed to be loopback device), was to reach inside the loopback device, find the underlaying file, and call fallocate on it. Fortunately "xen-blkback: convert hole punching to discard request on loop devices" removes that use-case and we now based the discard support based on blk_queue_discard(q) and extract all appropriate parameters from the 'struct request_queue'. CC: Li Dongyang Acked-by: Jan Beulich [v1: Dropping pointless initializer and keeping blank line] [v2: Remove the kfree as it is not used anymore] Signed-off-by: Konrad Rzeszutek Wilk --- drivers/block/xen-blkback/blkback.c | 19 ++++---- drivers/block/xen-blkback/common.h | 6 --- drivers/block/xen-blkback/xenbus.c | 70 +++++++++++------------------ 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index 70caa8969972..73f196ca713f 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -398,21 +398,18 @@ static int dispatch_discard_io(struct xen_blkif *blkif, int err = 0; int status = BLKIF_RSP_OKAY; struct block_device *bdev = blkif->vbd.bdev; + unsigned long secure; blkif->st_ds_req++; xen_blkif_get(blkif); - if (blkif->blk_backend_type == BLKIF_BACKEND_PHY || - blkif->blk_backend_type == BLKIF_BACKEND_FILE) { - unsigned long secure = (blkif->vbd.discard_secure && - (req->u.discard.flag & BLKIF_DISCARD_SECURE)) ? - BLKDEV_DISCARD_SECURE : 0; - err = blkdev_issue_discard(bdev, - req->u.discard.sector_number, - req->u.discard.nr_sectors, - GFP_KERNEL, secure); - } else - err = -EOPNOTSUPP; + secure = (blkif->vbd.discard_secure && + (req->u.discard.flag & BLKIF_DISCARD_SECURE)) ? + BLKDEV_DISCARD_SECURE : 0; + + err = blkdev_issue_discard(bdev, req->u.discard.sector_number, + req->u.discard.nr_sectors, + GFP_KERNEL, secure); if (err == -EOPNOTSUPP) { pr_debug(DRV_PFX "discard op failed, not supported\n"); diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h index d0ee7edc9be8..773cf27dc23f 100644 --- a/drivers/block/xen-blkback/common.h +++ b/drivers/block/xen-blkback/common.h @@ -146,11 +146,6 @@ enum blkif_protocol { BLKIF_PROTOCOL_X86_64 = 3, }; -enum blkif_backend_type { - BLKIF_BACKEND_PHY = 1, - BLKIF_BACKEND_FILE = 2, -}; - struct xen_vbd { /* What the domain refers to this vbd as. */ blkif_vdev_t handle; @@ -177,7 +172,6 @@ struct xen_blkif { unsigned int irq; /* Comms information. */ enum blkif_protocol blk_protocol; - enum blkif_backend_type blk_backend_type; union blkif_back_rings blk_rings; void *blk_ring; /* The VBD attached to this interface. */ diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index 24a2fb57e5d0..d417c13e027e 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -390,61 +390,43 @@ int xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be) { struct xenbus_device *dev = be->dev; struct xen_blkif *blkif = be->blkif; - char *type; int err; int state = 0; + struct block_device *bdev = be->blkif->vbd.bdev; + struct request_queue *q = bdev_get_queue(bdev); - type = xenbus_read(XBT_NIL, dev->nodename, "type", NULL); - if (!IS_ERR(type)) { - if (strncmp(type, "file", 4) == 0) { - state = 1; - blkif->blk_backend_type = BLKIF_BACKEND_FILE; + if (blk_queue_discard(q)) { + err = xenbus_printf(xbt, dev->nodename, + "discard-granularity", "%u", + q->limits.discard_granularity); + if (err) { + xenbus_dev_fatal(dev, err, + "writing discard-granularity"); + goto out; } - if (strncmp(type, "phy", 3) == 0) { - struct block_device *bdev = be->blkif->vbd.bdev; - struct request_queue *q = bdev_get_queue(bdev); - if (blk_queue_discard(q)) { - err = xenbus_printf(xbt, dev->nodename, - "discard-granularity", "%u", - q->limits.discard_granularity); - if (err) { - xenbus_dev_fatal(dev, err, - "writing discard-granularity"); - goto kfree; - } - err = xenbus_printf(xbt, dev->nodename, - "discard-alignment", "%u", - q->limits.discard_alignment); - if (err) { - xenbus_dev_fatal(dev, err, - "writing discard-alignment"); - goto kfree; - } - state = 1; - blkif->blk_backend_type = BLKIF_BACKEND_PHY; - } - /* Optional. */ - err = xenbus_printf(xbt, dev->nodename, - "discard-secure", "%d", - blkif->vbd.discard_secure); - if (err) { - xenbus_dev_fatal(dev, err, + err = xenbus_printf(xbt, dev->nodename, + "discard-alignment", "%u", + q->limits.discard_alignment); + if (err) { + xenbus_dev_fatal(dev, err, + "writing discard-alignment"); + goto out; + } + state = 1; + /* Optional. */ + err = xenbus_printf(xbt, dev->nodename, + "discard-secure", "%d", + blkif->vbd.discard_secure); + if (err) { + xenbus_dev_fatal(dev, err, "writting discard-secure"); - goto kfree; - } + goto out; } - } else { - err = PTR_ERR(type); - xenbus_dev_fatal(dev, err, "reading type"); - goto out; } - err = xenbus_printf(xbt, dev->nodename, "feature-discard", "%d", state); if (err) xenbus_dev_fatal(dev, err, "writing feature-discard"); -kfree: - kfree(type); out: return err; } From 3389bb8bf76180eecaffdfa7dd5b35fa4a2ce9b5 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Wed, 14 Mar 2012 13:04:00 -0400 Subject: [PATCH 2/2] xen/blkback: Make optional features be really optional. They were using the xenbus_dev_fatal() function which would change the state of the connection immediately. Which is not what we want when we advertise optional features. So make 'feature-discard','feature-barrier','feature-flush-cache' optional. Suggested-by: Jan Beulich [v1: Made the discard function void and static] Signed-off-by: Konrad Rzeszutek Wilk --- drivers/block/xen-blkback/xenbus.c | 37 ++++++++++++------------------ 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index d417c13e027e..89860f34a7ec 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -381,12 +381,12 @@ int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt, err = xenbus_printf(xbt, dev->nodename, "feature-flush-cache", "%d", state); if (err) - xenbus_dev_fatal(dev, err, "writing feature-flush-cache"); + dev_warn(&dev->dev, "writing feature-flush-cache (%d)", err); return err; } -int xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be) +static void xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be) { struct xenbus_device *dev = be->dev; struct xen_blkif *blkif = be->blkif; @@ -400,17 +400,15 @@ int xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be) "discard-granularity", "%u", q->limits.discard_granularity); if (err) { - xenbus_dev_fatal(dev, err, - "writing discard-granularity"); - goto out; + dev_warn(&dev->dev, "writing discard-granularity (%d)", err); + return; } err = xenbus_printf(xbt, dev->nodename, "discard-alignment", "%u", q->limits.discard_alignment); if (err) { - xenbus_dev_fatal(dev, err, - "writing discard-alignment"); - goto out; + dev_warn(&dev->dev, "writing discard-alignment (%d)", err); + return; } state = 1; /* Optional. */ @@ -418,17 +416,14 @@ int xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be) "discard-secure", "%d", blkif->vbd.discard_secure); if (err) { - xenbus_dev_fatal(dev, err, - "writting discard-secure"); - goto out; + dev_warn(dev-dev, "writing discard-secure (%d)", err); + return; } } err = xenbus_printf(xbt, dev->nodename, "feature-discard", "%d", state); if (err) - xenbus_dev_fatal(dev, err, "writing feature-discard"); -out: - return err; + dev_warn(&dev->dev, "writing feature-discard (%d)", err); } int xen_blkbk_barrier(struct xenbus_transaction xbt, struct backend_info *be, int state) @@ -439,7 +434,7 @@ int xen_blkbk_barrier(struct xenbus_transaction xbt, err = xenbus_printf(xbt, dev->nodename, "feature-barrier", "%d", state); if (err) - xenbus_dev_fatal(dev, err, "writing feature-barrier"); + dev_warn(&dev->dev, "writing feature-barrier (%d)", err); return err; } @@ -671,14 +666,12 @@ again: return; } - err = xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); - if (err) - goto abort; - - err = xen_blkbk_discard(xbt, be); - /* If we can't advertise it is OK. */ - err = xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); + xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); + + xen_blkbk_discard(xbt, be); + + xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); err = xenbus_printf(xbt, dev->nodename, "sectors", "%llu", (unsigned long long)vbd_sz(&be->blkif->vbd));