* [PATCH] minor fixes to xen-blkback (v1). @ 2012-03-14 18:10 Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 1/2] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 2/2] xen/blkback: Make optional features be really optional Konrad Rzeszutek Wilk 0 siblings, 2 replies; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-03-14 18:10 UTC (permalink / raw) To: JBeulich, linux-kernel, axboe; +Cc: xen-devel One of the patches I had already posted - just redid it per Jan's suggestion. The other patch is based on Jan suggestion. If there are no conerns I have put them in the usual branch: git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/for-jens-3.4 and will send out shortly a git pull. drivers/block/xen-blkback/blkback.c | 19 +++----- drivers/block/xen-blkback/common.h | 6 --- drivers/block/xen-blkback/xenbus.c | 83 +++++++++++++--------------------- 3 files changed, 40 insertions(+), 68 deletions(-) ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] xen/blkback: Squash the discard support for 'file' and 'phy' type. 2012-03-14 18:10 [PATCH] minor fixes to xen-blkback (v1) Konrad Rzeszutek Wilk @ 2012-03-14 18:10 ` Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 2/2] xen/blkback: Make optional features be really optional Konrad Rzeszutek Wilk 1 sibling, 0 replies; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-03-14 18:10 UTC (permalink / raw) To: JBeulich, linux-kernel, axboe Cc: xen-devel, Konrad Rzeszutek Wilk, Li Dongyang 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 <lidongyang@novell.com> Acked-by: Jan Beulich <JBeulich@suse.com> [v1: Dropping pointless initializer and keeping blank line] [v2: Remove the kfree as it is not used anymore] Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- 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 70caa89..73f196c 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 d0ee7ed..773cf27 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 24a2fb5..d417c13 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; + } + 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; } - 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, + 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; } -- 1.7.9.48.g85da4d ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] xen/blkback: Make optional features be really optional. 2012-03-14 18:10 [PATCH] minor fixes to xen-blkback (v1) Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 1/2] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk @ 2012-03-14 18:10 ` Konrad Rzeszutek Wilk 2012-03-15 8:50 ` Jan Beulich 1 sibling, 1 reply; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-03-14 18:10 UTC (permalink / raw) To: JBeulich, linux-kernel, axboe; +Cc: xen-devel, Konrad Rzeszutek Wilk 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 <JBeulich@suse.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/block/xen-blkback/xenbus.c | 25 ++++++++++--------------- 1 files changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index d417c13..d90a39c 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -381,7 +381,7 @@ 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; } @@ -400,16 +400,14 @@ 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"); + dev_warn(&dev->dev, "writing discard-granularity (%d)", err); goto out; } err = xenbus_printf(xbt, dev->nodename, "discard-alignment", "%u", q->limits.discard_alignment); if (err) { - xenbus_dev_fatal(dev, err, - "writing discard-alignment"); + dev_warn(&dev->dev, "writing discard-alignment (%d)", err); goto out; } state = 1; @@ -418,15 +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"); + dev_warn(dev-dev, "writing discard-secure (%d)", err); goto out; } } err = xenbus_printf(xbt, dev->nodename, "feature-discard", "%d", state); if (err) - xenbus_dev_fatal(dev, err, "writing feature-discard"); + dev_warn(&dev->dev, "writing feature-discard (%d)", err); out: return err; } @@ -439,7 +436,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 +668,12 @@ again: return; } - err = xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); - if (err) - goto abort; + /* If we can't advertise it is OK. */ + (void)xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); - err = xen_blkbk_discard(xbt, be); + (void)xen_blkbk_discard(xbt, be); - /* If we can't advertise it is OK. */ - err = xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); + (void)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)); -- 1.7.9.48.g85da4d ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] xen/blkback: Make optional features be really optional. 2012-03-14 18:10 ` [PATCH 2/2] xen/blkback: Make optional features be really optional Konrad Rzeszutek Wilk @ 2012-03-15 8:50 ` Jan Beulich 0 siblings, 0 replies; 4+ messages in thread From: Jan Beulich @ 2012-03-15 8:50 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: axboe, xen-devel, linux-kernel >>> On 14.03.12 at 19:10, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > 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 <JBeulich@suse.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > drivers/block/xen-blkback/xenbus.c | 25 ++++++++++--------------- > 1 files changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/block/xen-blkback/xenbus.c > b/drivers/block/xen-blkback/xenbus.c > index d417c13..d90a39c 100644 > --- a/drivers/block/xen-blkback/xenbus.c > +++ b/drivers/block/xen-blkback/xenbus.c > @@ -381,7 +381,7 @@ 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); I'm really not certain whether dev_warn() or xenbus_dev_error() is better here. The latter would also make the error visible in xenstore, but whether that's good, superfluous, or outright bad I'm unsure about. > > return err; > } > @@ -400,16 +400,14 @@ 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"); > + dev_warn(&dev->dev, "writing discard-granularity (%d)", err); > goto out; > } > err = xenbus_printf(xbt, dev->nodename, > "discard-alignment", "%u", > q->limits.discard_alignment); > if (err) { > - xenbus_dev_fatal(dev, err, > - "writing discard-alignment"); > + dev_warn(&dev->dev, "writing discard-alignment (%d)", err); > goto out; > } > state = 1; > @@ -418,15 +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"); > + dev_warn(dev-dev, "writing discard-secure (%d)", err); > goto out; > } > } > err = xenbus_printf(xbt, dev->nodename, "feature-discard", > "%d", state); > if (err) > - xenbus_dev_fatal(dev, err, "writing feature-discard"); > + dev_warn(&dev->dev, "writing feature-discard (%d)", err); > out: > return err; > } > @@ -439,7 +436,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 +668,12 @@ again: > return; > } > > - err = xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); > - if (err) > - goto abort; > + /* If we can't advertise it is OK. */ > + (void)xen_blkbk_flush_diskcache(xbt, be, be->blkif->vbd.flush_support); > > - err = xen_blkbk_discard(xbt, be); > + (void)xen_blkbk_discard(xbt, be); > > - /* If we can't advertise it is OK. */ > - err = xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); > + (void)xen_blkbk_barrier(xbt, be, be->blkif->vbd.flush_support); The casts here are bogus in any case. Even more, the functions can now easily be declared to return "void", as they don't have a caller caring for their return value anymore (xen_blkbk_discard() should even be "static"). Jan > > err = xenbus_printf(xbt, dev->nodename, "sectors", "%llu", > (unsigned long long)vbd_sz(&be->blkif->vbd)); ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-03-15 8:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-14 18:10 [PATCH] minor fixes to xen-blkback (v1) Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 1/2] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk 2012-03-14 18:10 ` [PATCH 2/2] xen/blkback: Make optional features be really optional Konrad Rzeszutek Wilk 2012-03-15 8:50 ` Jan Beulich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).