* [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
@ 2012-03-13 22:52 Konrad Rzeszutek Wilk
2012-03-14 8:33 ` [Xen-devel] " Ian Campbell
2012-03-14 9:48 ` Jan Beulich
0 siblings, 2 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-13 22:52 UTC (permalink / raw)
To: linux-kernel, axboe, JBeulich
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>
CC: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/block/xen-blkback/blkback.c | 20 ++++------
drivers/block/xen-blkback/common.h | 6 ---
drivers/block/xen-blkback/xenbus.c | 67 +++++++++++++---------------------
3 files changed, 34 insertions(+), 59 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 0088bf6..1a83f39 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -419,21 +419,17 @@ 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 = 0;
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..9aad6de 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -393,52 +393,37 @@ int xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info *be)
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 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;
}
- 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 kfree;
}
- } 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)
--
1.7.9.48.g85da4d
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
2012-03-13 22:52 [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk
@ 2012-03-14 8:33 ` Ian Campbell
2012-03-14 20:23 ` Konrad Rzeszutek Wilk
2012-03-14 9:48 ` Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2012-03-14 8:33 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel@vger.kernel.org, axboe@kernel.dk, JBeulich@suse.com,
xen-devel@lists.xensource.com, Li Dongyang
On Tue, 2012-03-13 at 22:52 +0000, Konrad Rzeszutek Wilk wrote:
> The only reason for the distinction was for the special case of
> 'file' (which is assumed to be loopback device)
Not related to this patch but how feasible would it be to make blkback
able to cope with files directly?
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
2012-03-13 22:52 [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk
2012-03-14 8:33 ` [Xen-devel] " Ian Campbell
@ 2012-03-14 9:48 ` Jan Beulich
2012-03-14 16:03 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-03-14 9:48 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: axboe, xen-devel, Li Dongyang, linux-kernel
>>> On 13.03.12 at 23:52, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 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>
> CC: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(with a few minor remarks below)
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -419,21 +419,17 @@ 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 = 0;
Mind keeping the blank line and dropping the pointless initializer (which
future gcc is likely going to be warning about)?
> 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");
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -393,52 +393,37 @@ int xen_blkbk_discard(struct xenbus_transaction xbt,
> struct backend_info *be)
> 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 kfree;
Unrelated to the patch here, but failure to write any sort of extension
data shouldn't be considered fatal - the backend can well work without
these, and it should be left to the frontend to decide whether it wants
to live without the unavailable extensions.
Jan
> + }
> + 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;
> }
> - 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 kfree;
> }
> - } 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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
2012-03-14 9:48 ` Jan Beulich
@ 2012-03-14 16:03 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-14 16:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: axboe, xen-devel, Li Dongyang, linux-kernel
On Wed, Mar 14, 2012 at 09:48:21AM +0000, Jan Beulich wrote:
> >>> On 13.03.12 at 23:52, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > 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>
> > CC: Jan Beulich <JBeulich@suse.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> (with a few minor remarks below)
>
> > --- a/drivers/block/xen-blkback/blkback.c
> > +++ b/drivers/block/xen-blkback/blkback.c
> > @@ -419,21 +419,17 @@ 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 = 0;
>
> Mind keeping the blank line and dropping the pointless initializer (which
> future gcc is likely going to be warning about)?
<nods>
>
> > 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");
> > --- a/drivers/block/xen-blkback/xenbus.c
> > +++ b/drivers/block/xen-blkback/xenbus.c
> > @@ -393,52 +393,37 @@ int xen_blkbk_discard(struct xenbus_transaction xbt,
> > struct backend_info *be)
> > 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 kfree;
>
> Unrelated to the patch here, but failure to write any sort of extension
> data shouldn't be considered fatal - the backend can well work without
> these, and it should be left to the frontend to decide whether it wants
> to live without the unavailable extensions.
<nods>. Let me whip up another patch that removes these 'fatal' cases
and is based on this one.
>
> Jan
>
> > + }
> > + 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;
> > }
> > - 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 kfree;
> > }
> > - } 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)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
2012-03-14 8:33 ` [Xen-devel] " Ian Campbell
@ 2012-03-14 20:23 ` Konrad Rzeszutek Wilk
2012-03-15 7:43 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-14 20:23 UTC (permalink / raw)
To: Ian Campbell
Cc: axboe@kernel.dk, xen-devel@lists.xensource.com, Li Dongyang,
linux-kernel@vger.kernel.org, JBeulich@suse.com
On Wed, Mar 14, 2012 at 08:33:55AM +0000, Ian Campbell wrote:
> On Tue, 2012-03-13 at 22:52 +0000, Konrad Rzeszutek Wilk wrote:
> > The only reason for the distinction was for the special case of
> > 'file' (which is assumed to be loopback device)
>
> Not related to this patch but how feasible would it be to make blkback
> able to cope with files directly?
Intersting thought. I am not sure... but an interesting thought. Would you
have Daniel's private email? We could email him for thoughts.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type.
2012-03-14 20:23 ` Konrad Rzeszutek Wilk
@ 2012-03-15 7:43 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2012-03-15 7:43 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: axboe@kernel.dk, xen-devel@lists.xensource.com, Li Dongyang,
linux-kernel@vger.kernel.org, JBeulich@suse.com
On Wed, 2012-03-14 at 20:23 +0000, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 14, 2012 at 08:33:55AM +0000, Ian Campbell wrote:
> > On Tue, 2012-03-13 at 22:52 +0000, Konrad Rzeszutek Wilk wrote:
> > > The only reason for the distinction was for the special case of
> > > 'file' (which is assumed to be loopback device)
> >
> > Not related to this patch but how feasible would it be to make blkback
> > able to cope with files directly?
>
> Intersting thought. I am not sure... but an interesting thought. Would you
> have Daniel's private email? We could email him for thoughts.
I'm afraid I don't have any contact info for Daniel these days.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-15 7:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-13 22:52 [PATCH] xen/blkback: Squash the discard support for 'file' and 'phy' type Konrad Rzeszutek Wilk
2012-03-14 8:33 ` [Xen-devel] " Ian Campbell
2012-03-14 20:23 ` Konrad Rzeszutek Wilk
2012-03-15 7:43 ` Ian Campbell
2012-03-14 9:48 ` Jan Beulich
2012-03-14 16:03 ` Konrad Rzeszutek Wilk
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).