On Fri, Sep 02, 2011 at 02:30:03PM +0800, Li Dongyang wrote:
> On Thu, Sep 1, 2011 at 11:25 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx> wrote:
> > On Thu, Sep 01, 2011 at 06:39:09PM +0800, Li Dongyang wrote:
> >> The blkfront driver now will read discard related nodes from xenstore,
> >> and set up the request queue, then we can forward the
> >> discard requests to backend driver.
> >
> >
> > A better description is as follow:
> >
> > xen-blkfront: Handle discard requests.
> >
> > If the backend advertises 'feature-discard', then interrogate
> > the backend for alignment, granularity, and max discard block size.
> > Setup the request queue with the appropiate values and send the
> > discard operation as required.
> >
> thanks for the description, way better than mine :-)
OK. Lets use that.
.. snip..
> >> + if (info->feature_discard) {
> >> + queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, rq);
> >> + blk_queue_max_discard_sectors(rq, get_capacity(gd));
> >
> > This is not correct. I took a look at the SCSI support for this
> > ('sd_config_discard') and if you look there carefully you will see that
> > the value can be 64KB for example.
> well I don't think so, max_discard_sectors are used to split the bio,
> make them not to
> exceed the max_discard_sectors, see blkdev_issue_discard, and
> that's fine if we use the capacity as max_discard_sectors
> in guest: for file backend, there's no need for max_discard_sectors,
> and for phy backend, we'll make the blkfront sent out a single big
> discard request,
Right, one sector for the whole disk size.
> and when we deal with that in blkback by calling blkdev_issue_discard,
> the real max_discard_sectors
> from phy dev will be used, and take care of splitting bios, Thanks
Ah, you are right:
if (nr_sects > max_discard_sectors) {
bio->bi_size = max_discard_sectors << 9;
nr_sects -= max_discard_sectors;
sector += max_discard_sectors;
} else {
handles when that value (nr_sects == get_capacity(x))
is extremely large.
OK, that was the only issue I had - and that has been resolved.
So stuck your patchset series in my tree - but they are not yet
visisble at kernel.org
>
> >
> > You need to provide a mechanism similar to 'discard-*' to fetch that data
> > from the backend.
> >
> >> + rq->limits.discard_granularity = info->discard_granularity;
> >> + rq->limits.discard_alignment = info->discard_alignment;
> >> + }
> >> +
> >> /* Hard sector size and max sectors impersonate the equiv. hardware.
> >> */
> >> blk_queue_logical_block_size(rq, sector_size);
> >> blk_queue_max_hw_sectors(rq, 512);
> >> @@ -722,6 +740,19 @@ static irqreturn_t blkif_interrupt(int irq, void
> >> *dev_id)
> >>
> >> error = (bret->status == BLKIF_RSP_OKAY) ? 0 : -EIO;
> >> switch (bret->operation) {
> >> + case BLKIF_OP_DISCARD:
> >> + if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
> >> + struct request_queue *rq = info->rq;
> >> + printk(KERN_WARNING "blkfront: %s: discard
> >> op failed\n",
> >> + info->gd->disk_name);
> >> + error = -EOPNOTSUPP;
> >> + info->feature_discard = 0;
> >> + spin_lock(rq->queue_lock);
> >> + queue_flag_clear(QUEUE_FLAG_DISCARD, rq);
> >> + spin_unlock(rq->queue_lock);
> >> + }
> >> + __blk_end_request_all(req, error);
> >> + break;
> >> case BLKIF_OP_FLUSH_DISKCACHE:
> >> case BLKIF_OP_WRITE_BARRIER:
> >> if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
> >> @@ -1098,6 +1129,33 @@ blkfront_closing(struct blkfront_info *info)
> >> bdput(bdev);
> >> }
> >>
> >> +static void blkfront_setup_discard(struct blkfront_info *info)
> >> +{
> >> + int err;
> >> + char *type;
> >> + unsigned int discard_granularity;
> >> + unsigned int discard_alignment;
> >> +
> >> + type = xenbus_read(XBT_NIL, info->xbdev->otherend, "type", NULL);
> >> + if (IS_ERR(type))
> >> + return;
> >> +
> >> + if (strncmp(type, "phy", 3) == 0) {
> >> + err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> >> + "discard-granularity", "%u", &discard_granularity,
> >> + "discard-alignment", "%u", &discard_alignment,
> >> + NULL);
> >> + if (!err) {
> >> + info->feature_discard = 1;
> >> + info->discard_granularity = discard_granularity;
> >> + info->discard_alignment = discard_alignment;
> >> + }
> >> + } else if (strncmp(type, "file", 4) == 0)
> >> + info->feature_discard = 1;
> >> +
> >> + kfree(type);
> >> +}
> >> +
> >> /*
> >> * Invoked when the backend is finally 'ready' (and has told produced
> >> * the details about the physical device - #sectors, size, etc).
> >> @@ -1108,7 +1166,7 @@ static void blkfront_connect(struct blkfront_info
> >> *info)
> >> unsigned long sector_size;
> >> unsigned int binfo;
> >> int err;
> >> - int barrier, flush;
> >> + int barrier, flush, discard;
> >>
> >> switch (info->connected) {
> >> case BLKIF_STATE_CONNECTED:
> >> @@ -1178,7 +1236,14 @@ static void blkfront_connect(struct blkfront_info
> >> *info)
> >> info->feature_flush = REQ_FLUSH;
> >> info->flush_op = BLKIF_OP_FLUSH_DISKCACHE;
> >> }
> >> -
> >> +
> >> + err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> >> + "feature-discard", "%d", &discard,
> >> + NULL);
> >> +
> >> + if (!err && discard)
> >> + blkfront_setup_discard(info);
> >> +
> >> err = xlvbd_alloc_gendisk(sectors, info, binfo, sector_size);
> >> if (err) {
> >> xenbus_dev_fatal(info->xbdev, err, "xlvbd_add at %s",
> >> --
> >> 1.7.6
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home