From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Owen Smith <owen.smith@citrix.com>
Cc: Xen Devel <xen-devel@lists.xensource.com>
Subject: Re: [PATCH] add trim command to blkback interface
Date: Mon, 06 Dec 2010 12:26:22 -0800 [thread overview]
Message-ID: <4CFD46EE.5070203@goop.org> (raw)
In-Reply-To: <DECC4DEE86A8074C9ABE8DFD330CB6E38D07A16D3D@LONPMAILBOX01.citrite.net>
On 12/06/2010 03:28 AM, Owen Smith wrote:
>
> [PATCH] add trim command to blkback interface
>
>
>
> This patch adds the trim operation to the blkback ring protocol.
>
> The ring protocol is extended by unioning the current read/write/write
> barrier specific fields with fields specific for trim, and allowing
> further expansion with additional union members, providing the
> structure size and alignment do not change.
>
>
>
> Blkback will respond to trim operations with a BLKIF_RSP_EOPNOTSUPP,
> to avoid using the default response of writing an error log entry and
> returning BLKIF_RSP_ERROR.
>
>
>
> Trim commands are passed with sector_number as the sector index to
> begin trim operations at, and nr_sectors as the number of sectors to
> be trimmed. These sectors should be trimmed if the underlying block
> device supports trimming. More information about trim commands:
>
> http://t13.org/Documents/UploadedDocuments/docs2008/e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc
>
Aside from the comments in my reply to Jan's mail, I can't actually get
an applyable patch out of this; its all mashed up with
quoted-printable. Could you repost as a plain-text attachment if you
can't convince your mailer to send a plain-text email?
Thanks,
J
>
>
> This patch is intended to fix an interface, not supply the
> implementation of trim in the backend drivers.
>
>
>
> Signed-off-by: Owen Smith <owen.smith@citrix.com>
>
>
>
>
>
> diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
>
> index 0bef445..bd09512 100644
>
> --- a/drivers/xen/blkback/blkback.c
>
> +++ b/drivers/xen/blkback/blkback.c
>
> @@ -367,6 +367,11 @@ static int do_block_io_op(blkif_t *blkif)
>
> blkif->st_wr_req++;
>
>
> dispatch_rw_block_io(blkif, &req, pending_req);
>
> break;
>
> + case BLKIF_OP_TRIM:
>
> + make_response(blkif,
> req.id, req.operation,
>
> +
> BLKIF_RSP_EOPNOTSUPP);
>
> + free_req(pending_req);
>
> + break;
>
> default:
>
> /* A good sign
> something is wrong: sleep for a while to
>
> * avoid excessive CPU
> consumption by a bad guest. */
>
> @@ -424,7 +429,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
> }
>
> preq.dev = req->handle;
>
> - preq.sector_number = req->sector_number;
>
> + preq.sector_number = req->rw.sector_number;
>
> preq.nr_sects = 0;
>
> pending_req->blkif = blkif;
>
> @@ -436,11 +441,11 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
> for (i = 0; i < nseg; i++) {
>
> uint32_t flags;
>
> - seg[i].nsec = req->seg[i].last_sect -
>
> - req->seg[i].first_sect + 1;
>
> + seg[i].nsec = req->rw.seg[i].last_sect -
>
> +
> req->rw.seg[i].first_sect + 1;
>
> - if ((req->seg[i].last_sect >=
> (PAGE_SIZE >> 9)) ||
>
> - (req->seg[i].last_sect <
> req->seg[i].first_sect))
>
> + if ((req->rw.seg[i].last_sect >=
> (PAGE_SIZE >> 9)) ||
>
> + (req->rw.seg[i].last_sect <
> req->rw.seg[i].first_sect))
>
> goto fail_response;
>
> preq.nr_sects += seg[i].nsec;
>
> @@ -448,7 +453,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
> if (operation != READ)
>
> flags |= GNTMAP_readonly;
>
> gnttab_set_map_op(&map[i],
> vaddr(pending_req, i), flags,
>
> -
> req->seg[i].gref, blkif->domid);
>
> +
> req->rw.seg[i].gref, blkif->domid);
>
> }
>
> ret =
> HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg);
>
> @@ -466,11 +471,11 @@ static void dispatch_rw_block_io(blkif_t *blkif,
>
>
> page_to_pfn(pending_page(pending_req, i)),
>
>
> FOREIGN_FRAME(map[i].dev_bus_addr >> PAGE_SHIFT));
>
> seg[i].buf = map[i].dev_bus_addr |
>
> - (req->seg[i].first_sect
> << 9);
>
> +
> (req->rw.seg[i].first_sect << 9);
>
>
> blkback_pagemap_set(vaddr_pagenr(pending_req, i),
>
>
> pending_page(pending_req, i),
>
>
> blkif->domid, req->handle,
>
> -
> req->seg[i].gref);
>
> +
> req->rw.seg[i].gref);
>
> pending_handle(pending_req, i) =
> map[i].handle;
>
> }
>
> diff --git a/include/xen/blkif.h b/include/xen/blkif.h
>
> index 7172081..e727e5d 100644
>
> --- a/include/xen/blkif.h
>
> +++ b/include/xen/blkif.h
>
> @@ -44,8 +44,18 @@ struct blkif_x86_32_request {
>
> uint8_t nr_segments; /* number of
> segments */
>
> blkif_vdev_t handle; /* only for read/write
> requests */
>
> uint64_t id; /* private guest value,
> echoed in resp */
>
> - blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only) */
>
> - struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> + union {
>
> + struct blkif_x86_32_request_rw {
>
> + blkif_sector_t
> sector_number;/* start sector idx on disk (r/w only) */
>
> + struct
> blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> + } rw;
>
> +
>
> + struct blkif_x86_32_request_trim {
>
> + blkif_sector_t
> sector_number;
>
> + uint64_t nr_sectors;
>
> + } trim;
>
> + };
>
> };
>
> struct blkif_x86_32_response {
>
> uint64_t id; /* copied from request */
>
> @@ -62,8 +72,18 @@ struct blkif_x86_64_request {
>
> uint8_t nr_segments; /* number of
> segments */
>
> blkif_vdev_t handle; /* only for read/write
> requests */
>
> uint64_t __attribute__((__aligned__(8))) id;
>
> - blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only) */
>
> - struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> + union {
>
> + struct blkif_x86_64_request_rw {
>
> + blkif_sector_t
> sector_number;/* start sector idx on disk (r/w only) */
>
> + struct
> blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> + } rw;
>
> +
>
> + struct blkif_x86_64_request_trim {
>
> + blkif_sector_t
> sector_number;
>
> + uint64_t nr_sectors;
>
> + } trim;
>
> + };
>
> };
>
> struct blkif_x86_64_response {
>
> uint64_t __attribute__((__aligned__(8))) id;
>
> @@ -97,12 +117,24 @@ static void inline blkif_get_x86_32_req(struct
> blkif_request *dst, struct blkif_
>
> dst->nr_segments = src->nr_segments;
>
> dst->handle = src->handle;
>
> dst->id = src->id;
>
> - dst->sector_number = src->sector_number;
>
> - barrier();
>
> - if (n > dst->nr_segments)
>
> - n = dst->nr_segments;
>
> - for (i = 0; i < n; i++)
>
> - dst->seg[i] = src->seg[i];
>
> + switch (src->operation) {
>
> + case BLKIF_OP_READ:
>
> + case BLKIF_OP_WRITE:
>
> + case BLKIF_OP_WRITE_BARRIER:
>
> + dst->rw.sector_number =
> src->rw.sector_number;
>
> + barrier();
>
> + if (n > dst->nr_segments)
>
> + n = dst->nr_segments;
>
> + for (i = 0; i < n; i++)
>
> + dst->rw.seg[i] =
> src->rw.seg[i];
>
> + break;
>
> + case BLKIF_OP_TRIM:
>
> + dst->trim.sector_number =
> src->trim.sector_number;
>
> + dst->trim.nr_sectors = src->trim.nr_sectors;
>
> + break;
>
> + default:
>
> + break;
>
> + }
>
> }
>
> static void inline blkif_get_x86_64_req(struct blkif_request *dst,
> struct blkif_x86_64_request *src)
>
> @@ -112,12 +144,24 @@ static void inline blkif_get_x86_64_req(struct
> blkif_request *dst, struct blkif_
>
> dst->nr_segments = src->nr_segments;
>
> dst->handle = src->handle;
>
> dst->id = src->id;
>
> - dst->sector_number = src->sector_number;
>
> - barrier();
>
> - if (n > dst->nr_segments)
>
> - n = dst->nr_segments;
>
> - for (i = 0; i < n; i++)
>
> - dst->seg[i] = src->seg[i];
>
> + switch (src->operation) {
>
> + case BLKIF_OP_READ:
>
> + case BLKIF_OP_WRITE:
>
> + case BLKIF_OP_WRITE_BARRIER:
>
> + dst->rw.sector_number =
> src->rw.sector_number;
>
> + barrier();
>
> + if (n > dst->nr_segments)
>
> + n = dst->nr_segments;
>
> + for (i = 0; i < n; i++)
>
> + dst->rw.seg[i] =
> src->rw.seg[i];
>
> + break;
>
> + case BLKIF_OP_TRIM:
>
> + dst->trim.sector_number =
> src->trim.sector_number;
>
> + dst->trim.nr_sectors = src->trim.nr_sectors;
>
> + break;
>
> + default:
>
> + break;
>
> + }
>
> }
>
> #endif /* __XEN_BLKIF_H__ */
>
> diff --git a/include/xen/interface/io/blkif.h
> b/include/xen/interface/io/blkif.h
>
> index 68dd2b4..54bb598 100644
>
> --- a/include/xen/interface/io/blkif.h
>
> +++ b/include/xen/interface/io/blkif.h
>
> @@ -43,6 +43,17 @@ typedef uint64_t blkif_sector_t;
>
> * create the "feature-barrier" node!
>
> */
>
> #define BLKIF_OP_WRITE_BARRIER 2
>
> +/*
>
> + * Recognised only if "feature-trim" is present in backend xenbus info.
>
> + * The "feature_trim" node contains a boolean indicating whether trim
>
> + * requests are likely to succeed or fail. Either way, a trim request
>
> + * may fail at any time with BLKIF_RSP_EOPNOTSUPP if it is unsupported by
>
> + * the underlying block-device hardware. The boolean simply indicates
> whether
>
> + * or not it is worthwhile for the frontend to attempt trim requests.
>
> + * If a backend does not recognise BLKIF_OP_TRIM, it should *not*
>
> + * create the "feature-trim" node!
>
> + */
>
> +#define BLKIF_OP_TRIM 4
>
> /*
>
> * Maximum scatter/gather segments per request.
>
> @@ -56,13 +67,23 @@ struct blkif_request {
>
> uint8_t nr_segments; /* number of
> segments */
>
> blkif_vdev_t handle; /* only for read/write
> requests */
>
> uint64_t id; /* private guest value,
> echoed in resp */
>
> - blkif_sector_t sector_number;/* start sector idx on
> disk (r/w only) */
>
> - struct blkif_request_segment {
>
> - grant_ref_t gref; /* reference
> to I/O buffer frame */
>
> - /* @first_sect: first sector in frame
> to transfer (inclusive). */
>
> - /* @last_sect: last sector in frame to
> transfer (inclusive). */
>
> - uint8_t first_sect, last_sect;
>
> - } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> +
>
> + union {
>
> + struct blkif_request_rw {
>
> + blkif_sector_t
> sector_number;/* start sector idx on disk (r/w only) */
>
> + struct
> blkif_request_segment {
>
> +
> grant_ref_t gref; /* reference to I/O buffer frame */
>
> + /*
> @first_sect: first sector in frame to transfer (inclusive). */
>
> + /*
> @last_sect: last sector in frame to transfer (inclusive). */
>
> +
> uint8_t first_sect, last_sect;
>
> + }
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>
> + } rw;
>
> +
>
> + struct blkif_request_trim {
>
> + blkif_sector_t sector_number;
>
> + uint64_t nr_sectors;
>
> + } trim;
>
> + };
>
> };
>
> struct blkif_response {
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
prev parent reply other threads:[~2010-12-06 20:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-06 11:28 [PATCH] add trim command to blkback interface Owen Smith
2010-12-06 11:53 ` Jan Beulich
2010-12-06 18:35 ` Jeremy Fitzhardinge
2010-12-07 10:06 ` Paul Durrant
2010-12-07 18:12 ` Jeremy Fitzhardinge
2010-12-06 20:26 ` Jeremy Fitzhardinge [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CFD46EE.5070203@goop.org \
--to=jeremy@goop.org \
--cc=owen.smith@citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.