[PATCH 0/2 0/2] Add Trim operation

[PATCH 0/2 0/2] Add Trim operation

[Owen Smith]

This set of patches for xen-unstable and the pvops dom0 kernel extend the 
current block device shared ring and add the trim operation interface.
The first patch in both series moves the request specific fields to a union.
The second patch in both series adds the trim operation.

xen-unstable
------------
1/2 : reorganises the block ring protocol to move the operation specific
      fields to a union.
2/2 : adds the trim operation definition and structure to the union.
               
pvops dom0
----------
1/2 : reorganises the block ring protocol to move the operation specific
      fields to a union.
2/2 : adds the trim operation definition and structure to the union. Also
      adds a BLKIF_RSP_EOPNOTSUPP response in blkback for the trim operation.

[PATCH 1/2 xen/stable-2.6.32.x] Union the blkif_request request specific fields

[Owen Smith]

Prepare for extending the block device ring to allow request
specific fields, by moving the request specific fields for
reads, writes and barrier requests to a union member

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 drivers/block/xen-blkfront.c     |    8 ++++----
 drivers/xen/blkback/blkback.c    |   16 ++++++++--------
 include/xen/blkif.h              |    8 ++++----
 include/xen/interface/io/blkif.h |   16 +++++++++++-----
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 44059e6..3316dc7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -286,7 +286,7 @@ static int blkif_queue_request(struct request *req)
 	info->shadow[id].request = (unsigned long)req;
 
 	ring_req->id = id;
-	ring_req->sector_number = (blkif_sector_t)blk_rq_pos(req);
+	ring_req->u.rw.sector_number = (blkif_sector_t)blk_rq_pos(req);
 	ring_req->handle = info->handle;
 
 	ring_req->operation = rq_data_dir(req) ?
@@ -312,7 +312,7 @@ static int blkif_queue_request(struct request *req)
 				rq_data_dir(req) );
 
 		info->shadow[id].frame[i] = mfn_to_pfn(buffer_mfn);
-		ring_req->seg[i] =
+		ring_req->u.rw.seg[i] =
 				(struct blkif_request_segment) {
 					.gref       = ref,
 					.first_sect = fsect,
@@ -692,7 +692,7 @@ static void blkif_completion(struct blk_shadow *s)
 {
 	int i;
 	for (i = 0; i < s->req.nr_segments; i++)
-		gnttab_end_foreign_access(s->req.seg[i].gref, 0, 0UL);
+		gnttab_end_foreign_access(s->req.u.rw.seg[i].gref, 0, 0UL);
 }
 
 static void
@@ -1010,7 +1010,7 @@ static int blkif_recover(struct blkfront_info *info)
 		/* Rewrite any grant references invalidated by susp/resume. */
 		for (j = 0; j < req->nr_segments; j++)
 			gnttab_grant_foreign_access_ref(
-				req->seg[j].gref,
+				req->u.rw.seg[j].gref,
 				info->xbdev->otherend_id,
 				pfn_to_mfn(info->shadow[req->id].frame[j]),
 				rq_data_dir(
diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
index 0bef445..b45b21f 100644
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -424,7 +424,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
 	}
 
 	preq.dev           = req->handle;
-	preq.sector_number = req->sector_number;
+	preq.sector_number = req->u.rw.sector_number;
 	preq.nr_sects      = 0;
 
 	pending_req->blkif     = blkif;
@@ -436,11 +436,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->u.rw.seg[i].last_sect -
+			req->u.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->u.rw.seg[i].last_sect >= (PAGE_SIZE >> 9)) ||
+		    (req->u.rw.seg[i].last_sect < req->u.rw.seg[i].first_sect))
 			goto fail_response;
 		preq.nr_sects += seg[i].nsec;
 
@@ -448,7 +448,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->u.rw.seg[i].gref, blkif->domid);
 	}
 
 	ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg);
@@ -466,11 +466,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->u.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->u.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..71018e9 100644
--- a/include/xen/blkif.h
+++ b/include/xen/blkif.h
@@ -97,12 +97,12 @@ 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;
+	dst->u.rw.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];
+		dst->u.rw.seg[i] = src->seg[i];
 }
 
 static void inline blkif_get_x86_64_req(struct blkif_request *dst, struct blkif_x86_64_request *src)
@@ -112,12 +112,12 @@ 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;
+	dst->u.rw.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];
+		dst->u.rw.seg[i] = src->seg[i];
 }
 
 #endif /* __XEN_BLKIF_H__ */
diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index 68dd2b4..61e523a 100644
--- a/include/xen/interface/io/blkif.h
+++ b/include/xen/interface/io/blkif.h
@@ -51,11 +51,7 @@ typedef uint64_t blkif_sector_t;
  */
 #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11
 
-struct blkif_request {
-	uint8_t        operation;    /* BLKIF_OP_???                         */
-	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  */
+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        */
@@ -65,6 +61,16 @@ struct blkif_request {
 	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request {
+	uint8_t        operation;    /* BLKIF_OP_???                         */
+	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  */
+	union {
+		struct blkif_request_rw rw;
+	} u;
+};
+
 struct blkif_response {
 	uint64_t        id;              /* copied from request */
 	uint8_t         operation;       /* copied from request */
-- 
1.5.6.5

[PATCH 2/2 xen/stable-2.6.32.x] Add trim operation to xen block devices

[Owen Smith]

Defines the trim operation code, data structure and adds it to the
blk ring protocol. Blkback responds to trim operations with a
BLKIF_RSP_EOPNOTSUPP instead of using the default response, which logs
the operation as failed in the error log file.

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. The specified sectors should be trimmed if the underlying
block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP should be
returned. More information about trim operations:
http://t13.org/Documents/UploadedDocuments/docs2008/
      e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 drivers/xen/blkback/blkback.c    |    7 +++++++
 include/xen/interface/io/blkif.h |   17 +++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
index b45b21f..03cb8f6 100644
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -367,6 +367,13 @@ 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:
+			/* respond with BLKIF_RSP_EOPNOTSUPP to reduce logging
+			 * from default case */
+			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. */
diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index 61e523a..1098d0c 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 barrier
+ * 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.
@@ -61,6 +72,11 @@ struct blkif_request_rw {
 	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request_trim {
+	blkif_sector_t sector_number;
+	uint64_t nr_sectors;
+};
+
 struct blkif_request {
 	uint8_t        operation;    /* BLKIF_OP_???                         */
 	uint8_t        nr_segments;  /* number of segments                   */
@@ -68,6 +84,7 @@ struct blkif_request {
 	uint64_t       id;           /* private guest value, echoed in resp  */
 	union {
 		struct blkif_request_rw rw;
+		struct blkif_request_trim trim;
 	} u;
 };
 
-- 
1.5.6.5

[PATCH 1/2] Move the block request specific fields into a union

[Owen Smith]

Move the request specific fields into a union

Make the block ring interface extensible by moving the current
read/write fields into a union.

Signed-off-by: Owen Smith <owen.smith@citrix.com>

# HG changeset patch
# User Owen Smith <owen.smith@citrix.com>
# Date 1292938128 0
# Node ID 7015d7407eae8694ce1cc92e792b2087f109f6cb
# Parent  89116f28083f7d118a259c5bc684d1c4296d9cb3
* * *

diff -r 89116f28083f -r 7015d7407eae extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/extras/mini-os/blkfront.c	Tue Dec 21 13:28:48 2010 +0000
@@ -361,22 +361,22 @@
     req->nr_segments = n;
     req->handle = dev->handle;
     req->id = (uintptr_t) aiocbp;
-    req->sector_number = aiocbp->aio_offset / 512;
+    req->u.rw.sector_number = aiocbp->aio_offset / 512;
 
     for (j = 0; j < n; j++) {
-        req->seg[j].first_sect = 0;
-        req->seg[j].last_sect = PAGE_SIZE / 512 - 1;
+        req->u.rw.seg[j].first_sect = 0;
+        req->u.rw.seg[j].last_sect = PAGE_SIZE / 512 - 1;
     }
-    req->seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) / 512;
-    req->seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
+    req->u.rw.seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) / 512;
+    req->u.rw.seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
     for (j = 0; j < n; j++) {
 	uintptr_t data = start + j * PAGE_SIZE;
         if (!write) {
             /* Trigger CoW if needed */
-            *(char*)(data + (req->seg[j].first_sect << 9)) = 0;
+            *(char*)(data + (req->u.rw.seg[j].first_sect << 9)) = 0;
             barrier();
         }
-	aiocbp->gref[j] = req->seg[j].gref =
+	aiocbp->gref[j] = req->u.rw.seg[j].gref =
             gnttab_grant_access(dev->dom, virtual_to_mfn(data), write);
     }
 
@@ -432,7 +432,7 @@
     req->handle = dev->handle;
     req->id = id;
     /* Not needed anyway, but the backend will check it */
-    req->sector_number = 0;
+    req->u.rw.sector_number = 0;
     dev->ring.req_prod_pvt = i + 1;
     wmb();
     RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&dev->ring, notify);
diff -r 89116f28083f -r 7015d7407eae tools/blktap/drivers/tapdisk.c
--- a/tools/blktap/drivers/tapdisk.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap/drivers/tapdisk.c	Tue Dec 21 13:28:48 2010 +0000
@@ -528,10 +528,10 @@
 segment_start(blkif_request_t *req, int sidx)
 {
 	int i;
-	uint64_t start = req->sector_number;
+	uint64_t start = req->u.rw.sector_number;
 
 	for (i = 0; i < sidx; i++) 
-		start += (req->seg[i].last_sect - req->seg[i].first_sect + 1);
+		start += (req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1);
 
 	return start;
 }
@@ -600,13 +600,13 @@
 	struct disk_driver *parent = dd->next;
 	
 	seg_start = segment_start(req, sidx);
-	seg_end   = seg_start + req->seg[sidx].last_sect + 1;
+	seg_end   = seg_start + req->u.rw.seg[sidx].last_sect + 1;
 	
 	ASSERT(sector >= seg_start && sector + nr_secs <= seg_end);
 
 	page  = (char *)MMAP_VADDR(info->vstart, 
 				   (unsigned long)req->id, sidx);
-	page += (req->seg[sidx].first_sect << SECTOR_SHIFT);
+	page += (req->u.rw.seg[sidx].first_sect << SECTOR_SHIFT);
 	page += ((sector - seg_start) << SECTOR_SHIFT);
 
 	if (!parent) {
@@ -667,7 +667,7 @@
 			       req, sizeof(*req));
 			blkif->pending_list[idx].status = BLKIF_RSP_OKAY;
 			blkif->pending_list[idx].submitting = 1;
-			sector_nr = req->sector_number;
+			sector_nr = req->u.rw.sector_number;
 		}
 
 		if ((dd->flags & TD_RDONLY) && 
@@ -677,16 +677,16 @@
 		}
 
 		for (i = start_seg; i < req->nr_segments; i++) {
-			nsects = req->seg[i].last_sect - 
-				 req->seg[i].first_sect + 1;
+			nsects = req->u.rw.seg[i].last_sect - 
+				 req->u.rw.seg[i].first_sect + 1;
 	
-			if ((req->seg[i].last_sect >= page_size >> 9) ||
+			if ((req->u.rw.seg[i].last_sect >= page_size >> 9) ||
 			    (nsects <= 0))
 				continue;
 
 			page  = (char *)MMAP_VADDR(info->vstart, 
 						   (unsigned long)req->id, i);
-			page += (req->seg[i].first_sect << SECTOR_SHIFT);
+			page += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
 
 			if (sector_nr >= s->size) {
 				DPRINTF("Sector request failed:\n");
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-diff.c
--- a/tools/blktap2/drivers/tapdisk-diff.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-diff.c	Tue Dec 21 13:28:48 2010 +0000
@@ -363,13 +363,13 @@
 	breq                = &sreq->blkif_req;
 	breq->id            = idx;
 	breq->nr_segments   = r->blkif_req.nr_segments;
-	breq->sector_number = r->blkif_req.sector_number;
+	breq->u.rw.sector_number = r->blkif_req.u.rw.sector_number;
 	breq->operation     = BLKIF_OP_READ;
 
 	for (int i = 0; i < r->blkif_req.nr_segments; i++) {
-		struct blkif_request_segment *seg = breq->seg + i;
-		seg->first_sect = r->blkif_req.seg[i].first_sect;
-		seg->last_sect  = r->blkif_req.seg[i].last_sect;
+		struct blkif_request_segment *seg = breq->u.rw.seg + i;
+		seg->first_sect = r->blkif_req.u.rw.seg[i].first_sect;
+		seg->last_sect  = r->blkif_req.u.rw.seg[i].last_sect;
 	}
 	s->cur += sreq->secs;
 
@@ -426,12 +426,12 @@
 		breq                = &sreq->blkif_req;
 		breq->id            = idx;
 		breq->nr_segments   = 0;
-		breq->sector_number = sreq->sec;
+		breq->u.rw.sector_number = sreq->sec;
 		breq->operation     = BLKIF_OP_READ;
 
 		for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
 			uint32_t secs;
-			struct blkif_request_segment *seg = breq->seg + i;
+			struct blkif_request_segment *seg = breq->u.rw.seg + i;
 
 			secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
 			secs = MIN(((blk + 1) << SPB_SHIFT) - s->cur, secs);
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-image.c
--- a/tools/blktap2/drivers/tapdisk-image.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-image.c	Tue Dec 21 13:28:48 2010 +0000
@@ -148,15 +148,15 @@
 	psize = getpagesize();
 
 	for (i = 0; i < req->nr_segments; i++) {
-		nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
+		nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
 		
-		if (req->seg[i].last_sect >= psize >> 9 || nsects <= 0)
+		if (req->u.rw.seg[i].last_sect >= psize >> 9 || nsects <= 0)
 			goto fail;
 
 		total += nsects;
 	}
 
-	if (req->sector_number + nsects > info->size)
+	if (req->u.rw.sector_number + nsects > info->size)
 		goto fail;
 
 	return 0;
@@ -164,6 +164,6 @@
 fail:
 	ERR(-EINVAL, "bad request on %s (%s, %"PRIu64"): id: %"PRIu64": %d at %"PRIu64,
 	    image->name, (rdonly ? "ro" : "rw"), info->size, req->id,
-	    req->operation, req->sector_number + total);
+	    req->operation, req->u.rw.sector_number + total);
 	return -EINVAL;
 }
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-stream.c
--- a/tools/blktap2/drivers/tapdisk-stream.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-stream.c	Tue Dec 21 13:28:48 2010 +0000
@@ -293,12 +293,12 @@
 		breq                = &sreq->blkif_req;
 		breq->id            = idx;
 		breq->nr_segments   = 0;
-		breq->sector_number = sreq->sec;
+		breq->u.rw.sector_number = sreq->sec;
 		breq->operation     = BLKIF_OP_READ;
 
 		for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
 			uint32_t secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
-			struct blkif_request_segment *seg = breq->seg + i;
+			struct blkif_request_segment *seg = breq->u.rw.seg + i;
 
 			if (!secs)
 				break;
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-vbd.c
--- a/tools/blktap2/drivers/tapdisk-vbd.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-vbd.c	Tue Dec 21 13:28:48 2010 +0000
@@ -1066,7 +1066,7 @@
 	rsp->status = vreq->status;
 
 	DBG(TLOG_DBG, "writing req %d, sec 0x%08"PRIx64", res %d to ring\n",
-	    (int)tmp.id, tmp.sector_number, vreq->status);
+	    (int)tmp.id, tmp.u.rw.sector_number, vreq->status);
 
 	if (rsp->status != BLKIF_RSP_OKAY)
 		ERR(EIO, "returning BLKIF_RSP %d", rsp->status);
@@ -1181,10 +1181,10 @@
 tapdisk_vbd_breq_get_sector(blkif_request_t *breq, td_request_t treq)
 {
     int seg, nsects; 
-    uint64_t sector_nr = breq->sector_number; 
+    uint64_t sector_nr = breq->u.rw.sector_number; 
     
     for(seg=0; seg < treq.sidx; seg++) {
-        nsects = breq->seg[seg].last_sect - breq->seg[seg].first_sect + 1;
+        nsects = breq->u.rw.seg[seg].last_sect - breq->u.rw.seg[seg].first_sect + 1;
         sector_nr += nsects;
     }
 
@@ -1222,8 +1222,8 @@
 			uint16_t uid  = image->memshr_id;
 			blkif_request_t *breq = &vreq->req;
 			uint64_t sec  = tapdisk_vbd_breq_get_sector(breq, treq);
-			int secs = breq->seg[treq.sidx].last_sect -
-			    breq->seg[treq.sidx].first_sect + 1;
+			int secs = breq->u.rw.seg[treq.sidx].last_sect -
+			    breq->u.rw.seg[treq.sidx].first_sect + 1;
 
 			if (hnd != 0)
 				memshr_vbd_complete_ro_request(hnd, uid,
@@ -1288,7 +1288,7 @@
 			blkif_request_t *breq = &vreq->req;
         
 			ret = memshr_vbd_issue_ro_request(treq.buf,
-			      breq->seg[seg].gref,
+			      breq->u.rw.seg[seg].gref,
 			      parent->memshr_id,
 			      treq.sec,
 			      treq.secs,
@@ -1366,7 +1366,7 @@
 	req       = &vreq->req;
 	id        = req->id;
 	ring      = &vbd->ring;
-	sector_nr = req->sector_number;
+	sector_nr = req->u.rw.sector_number;
 	image     = tapdisk_vbd_first_image(vbd);
 
 	vreq->submitting = 1;
@@ -1385,10 +1385,10 @@
 		goto fail;
 
 	for (i = 0; i < req->nr_segments; i++) {
-		nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
+		nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
 		page   = (char *)MMAP_VADDR(ring->vstart, 
 					   (unsigned long)req->id, i);
-		page  += (req->seg[i].first_sect << SECTOR_SHIFT);
+		page  += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
 
 		treq.id             = id;
 		treq.sidx           = i;
@@ -1482,7 +1482,7 @@
 		vreq->status = BLKIF_RSP_OKAY;
 		DBG(TLOG_DBG, "retry #%d of req %"PRIu64", "
 		    "sec 0x%08"PRIx64", nr_segs: %d\n", vreq->num_retries,
-		    vreq->req.id, vreq->req.sector_number,
+		    vreq->req.id, vreq->req.u.rw.sector_number,
 		    vreq->req.nr_segments);
 
 		err = tapdisk_vbd_issue_request(vbd, vreq);
diff -r 89116f28083f -r 7015d7407eae xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Wed Dec 08 10:46:31 2010 +0000
+++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
@@ -98,13 +98,19 @@
     uint8_t     first_sect, last_sect;
 };
 
+struct blkif_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];
+};
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     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_request_rw rw;
+    } u;
 };
 typedef struct blkif_request blkif_request_t;

[PATCH 2/2] Add trim operation to xen block devices

[Owen Smith]

Add Trim command interface

This patch adds the trim operation to the blkback ring protocol.

Signed-of-by: Owen Smith <owen.smith@citrix.com>

# HG changeset patch
# User Owen Smith <owen.smith@citrix.com>
# Date 1292938216 0
# Node ID 6be1a23977b82e44eba80feeca3fb77acf6bb2bb
# Parent  7015d7407eae8694ce1cc92e792b2087f109f6cb
imported patch add-trim

diff -r 7015d7407eae -r 6be1a23977b8 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
+++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:30:16 2010 +0000
@@ -76,6 +76,17 @@
  * "feature-flush-cache" node!
  */
 #define BLKIF_OP_FLUSH_DISKCACHE   3
+/*
+ * 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.
@@ -103,6 +114,11 @@
     struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request_trim {
+    blkif_sector_t sector_number;/* start sector idx on disk   */
+    uint64_t nr_sectors;
+};
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     uint8_t        nr_segments;  /* number of segments                   */
@@ -110,6 +126,7 @@
     uint64_t       id;           /* private guest value, echoed in resp  */
     union {
     	struct blkif_request_rw rw;
+    	struct blkif_request_trim trim;
     } u;
 };
 typedef struct blkif_request blkif_request_t;

Re: [PATCH 2/2] Add trim operation to xen block devices

[Keir Fraser]

On 21/12/2010 16:58, "Owen Smith" <owen.smith@citrix.com> wrote:

> +/*
> + * 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

What is a 'trim operation'? It's not documented in the patch description(s)
or in the protocol-defining header file.

 -- Keir

Re: [PATCH 2/2] Add trim operation to xen block devices

[Jan Beulich]

>>> On 21.12.10 at 17:58, Owen Smith <owen.smith@citrix.com> wrote:
> --- a/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
> +++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:30:16 2010 +0000
> @@ -76,6 +76,17 @@
>   * "feature-flush-cache" node!
>   */
>  #define BLKIF_OP_FLUSH_DISKCACHE   3
> +/*
> + * 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

I wonder if it would be possible to skip 4 here. We've been carrying
a patch to support packet commands (for CD-ROM support) for
quite a while, using 4 for BLKIF_OP_PACKET. I realize this should
have been presented on the list long ago, but unfortunately the
author never did and is no longer with the company. While I could
submit the kernel side patches, I wouldn't be able to advocate for
them, partly because I don't know all of the details, partly because
there are some rough edges (Linux-isms introduced into
xen/include/public/io/), and partly because backend support exists
only for blktap1 so far.

An alternative to submitting the full patch set would be to just
submit a patch adding the necessary definition here - given that
BLKIF_OP_FLUSH_DISKCACHE too looks like a half-baked thing
only (used exclusively in mini-os' blkfront and qemu's block-vbd;
I wasn't able to locate what backend might actually handle it),
would this be acceptable?

Thanks, Jan

Re: [PATCH 2/2] Add trim operation to xen block devices

[Ian Campbell]

On Wed, 2010-12-22 at 09:10 +0000, Jan Beulich wrote:
> >>> On 21.12.10 at 17:58, Owen Smith <owen.smith@citrix.com> wrote:
> > --- a/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
> > +++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:30:16 2010 +0000
> > @@ -76,6 +76,17 @@
> >   * "feature-flush-cache" node!
> >   */
> >  #define BLKIF_OP_FLUSH_DISKCACHE   3
> > +/*
> > + * 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
> 
> I wonder if it would be possible to skip 4 here. We've been carrying
> a patch to support packet commands (for CD-ROM support) for
> quite a while, using 4 for BLKIF_OP_PACKET. I realize this should
> have been presented on the list long ago, but unfortunately the
> author never did and is no longer with the company. While I could
> submit the kernel side patches, I wouldn't be able to advocate for
> them, partly because I don't know all of the details, partly because
> there are some rough edges (Linux-isms introduced into
> xen/include/public/io/), and partly because backend support exists
> only for blktap1 so far.
> 
> An alternative to submitting the full patch set would be to just
> submit a patch adding the necessary definition here[...]
> would this be acceptable?

Given that the horse has already left the stable and there isn't much we
can do about that I think adding some sort of placeholder for op 4 (even
just marking it as reserved) would be fine.

>  - given that
> BLKIF_OP_FLUSH_DISKCACHE too looks like a half-baked thing
> only (used exclusively in mini-os' blkfront and qemu's block-vbd;
> I wasn't able to locate what backend might actually handle it),

The commit which added it is signed-off-by Sun, so I guess solaris...

Ian.

> 
> Thanks, Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 0/2 0/2 v2] Add Trim operation

[Owen Smith]

* Updated value of BLKIF_OP_TRIM to 5 after comments from Jan Beulich
* Added explanation of trim operation after comments from Keir Fraser


This set of patches for xen-unstable and the pvops dom0 kernel extend the 
current block device shared ring and add the trim operation interface.
The first patch in both series moves the request specific fields to a union.
The second patch in both series adds the trim operation.

xen-unstable
------------
1/2 : reorganises the block ring protocol to move the operation specific
      fields to a union.
2/2 : adds the trim operation definition and structure to the union.
               
pvops dom0
----------
1/2 : reorganises the block ring protocol to move the operation specific
      fields to a union.
2/2 : adds the trim operation definition and structure to the union. Also
      adds a BLKIF_RSP_EOPNOTSUPP response in blkback for the trim operation.

[PATCH 1/2 xen/stable-2.6.32.x] Union the blkif_request request specific fields

[Owen Smith]

Prepare for extending the block device ring to allow request
specific fields, by moving the request specific fields for
reads, writes and barrier requests to a union member

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 drivers/block/xen-blkfront.c     |    8 ++++----
 drivers/xen/blkback/blkback.c    |   16 ++++++++--------
 include/xen/blkif.h              |    8 ++++----
 include/xen/interface/io/blkif.h |   16 +++++++++++-----
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 44059e6..3316dc7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -286,7 +286,7 @@ static int blkif_queue_request(struct request *req)
 	info->shadow[id].request = (unsigned long)req;
 
 	ring_req->id = id;
-	ring_req->sector_number = (blkif_sector_t)blk_rq_pos(req);
+	ring_req->u.rw.sector_number = (blkif_sector_t)blk_rq_pos(req);
 	ring_req->handle = info->handle;
 
 	ring_req->operation = rq_data_dir(req) ?
@@ -312,7 +312,7 @@ static int blkif_queue_request(struct request *req)
 				rq_data_dir(req) );
 
 		info->shadow[id].frame[i] = mfn_to_pfn(buffer_mfn);
-		ring_req->seg[i] =
+		ring_req->u.rw.seg[i] =
 				(struct blkif_request_segment) {
 					.gref       = ref,
 					.first_sect = fsect,
@@ -692,7 +692,7 @@ static void blkif_completion(struct blk_shadow *s)
 {
 	int i;
 	for (i = 0; i < s->req.nr_segments; i++)
-		gnttab_end_foreign_access(s->req.seg[i].gref, 0, 0UL);
+		gnttab_end_foreign_access(s->req.u.rw.seg[i].gref, 0, 0UL);
 }
 
 static void
@@ -1010,7 +1010,7 @@ static int blkif_recover(struct blkfront_info *info)
 		/* Rewrite any grant references invalidated by susp/resume. */
 		for (j = 0; j < req->nr_segments; j++)
 			gnttab_grant_foreign_access_ref(
-				req->seg[j].gref,
+				req->u.rw.seg[j].gref,
 				info->xbdev->otherend_id,
 				pfn_to_mfn(info->shadow[req->id].frame[j]),
 				rq_data_dir(
diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
index 0bef445..b45b21f 100644
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -424,7 +424,7 @@ static void dispatch_rw_block_io(blkif_t *blkif,
 	}
 
 	preq.dev           = req->handle;
-	preq.sector_number = req->sector_number;
+	preq.sector_number = req->u.rw.sector_number;
 	preq.nr_sects      = 0;
 
 	pending_req->blkif     = blkif;
@@ -436,11 +436,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->u.rw.seg[i].last_sect -
+			req->u.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->u.rw.seg[i].last_sect >= (PAGE_SIZE >> 9)) ||
+		    (req->u.rw.seg[i].last_sect < req->u.rw.seg[i].first_sect))
 			goto fail_response;
 		preq.nr_sects += seg[i].nsec;
 
@@ -448,7 +448,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->u.rw.seg[i].gref, blkif->domid);
 	}
 
 	ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg);
@@ -466,11 +466,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->u.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->u.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..71018e9 100644
--- a/include/xen/blkif.h
+++ b/include/xen/blkif.h
@@ -97,12 +97,12 @@ 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;
+	dst->u.rw.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];
+		dst->u.rw.seg[i] = src->seg[i];
 }
 
 static void inline blkif_get_x86_64_req(struct blkif_request *dst, struct blkif_x86_64_request *src)
@@ -112,12 +112,12 @@ 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;
+	dst->u.rw.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];
+		dst->u.rw.seg[i] = src->seg[i];
 }
 
 #endif /* __XEN_BLKIF_H__ */
diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index 68dd2b4..61e523a 100644
--- a/include/xen/interface/io/blkif.h
+++ b/include/xen/interface/io/blkif.h
@@ -51,11 +51,7 @@ typedef uint64_t blkif_sector_t;
  */
 #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11
 
-struct blkif_request {
-	uint8_t        operation;    /* BLKIF_OP_???                         */
-	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  */
+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        */
@@ -65,6 +61,16 @@ struct blkif_request {
 	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request {
+	uint8_t        operation;    /* BLKIF_OP_???                         */
+	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  */
+	union {
+		struct blkif_request_rw rw;
+	} u;
+};
+
 struct blkif_response {
 	uint64_t        id;              /* copied from request */
 	uint8_t         operation;       /* copied from request */
-- 
1.5.6.5

[PATCH 2/2 xen/stable-2.6.32.x v2] Add trim operation to xen block devices

[Owen Smith]

Defines the trim operation code, data structure and adds it to the
blk ring protocol. Blkback responds to trim operations with a
BLKIF_RSP_EOPNOTSUPP instead of using the default response, which logs
the operation as failed in the error log file.

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. The specified sectors should be trimmed if the underlying
block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP should be
returned. More information about trim operations:
http://t13.org/Documents/UploadedDocuments/docs2008/
      e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 drivers/xen/blkback/blkback.c    |    7 +++++++
 include/xen/interface/io/blkif.h |   17 +++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
index b45b21f..03cb8f6 100644
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -367,6 +367,13 @@ 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:
+			/* respond with BLKIF_RSP_EOPNOTSUPP to reduce logging
+			 * from default case */
+			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. */
diff --git a/include/xen/interface/io/blkif.h b/include/xen/interface/io/blkif.h
index 61e523a..1098d0c 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 barrier
+ * 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            5
 
 /*
  * Maximum scatter/gather segments per request.
@@ -61,6 +72,11 @@ struct blkif_request_rw {
 	} seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request_trim {
+	blkif_sector_t sector_number;
+	uint64_t nr_sectors;
+};
+
 struct blkif_request {
 	uint8_t        operation;    /* BLKIF_OP_???                         */
 	uint8_t        nr_segments;  /* number of segments                   */
@@ -68,6 +84,7 @@ struct blkif_request {
 	uint64_t       id;           /* private guest value, echoed in resp  */
 	union {
 		struct blkif_request_rw rw;
+		struct blkif_request_trim trim;
 	} u;
 };
 
-- 
1.5.6.5

[PATCH 1/2] Move the block request specific fields into a union

[Owen Smith]

Move the request specific fields into a union

Make the block ring interface extensible by moving the current
read/write fields into a union.

Signed-off-by: Owen Smith <owen.smith@citrix.com>

# HG changeset patch
# User Owen Smith <owen.smith@citrix.com>
# Date 1292938128 0
# Node ID 7015d7407eae8694ce1cc92e792b2087f109f6cb
# Parent  89116f28083f7d118a259c5bc684d1c4296d9cb3
* * *

diff -r 89116f28083f -r 7015d7407eae extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/extras/mini-os/blkfront.c	Tue Dec 21 13:28:48 2010 +0000
@@ -361,22 +361,22 @@
     req->nr_segments = n;
     req->handle = dev->handle;
     req->id = (uintptr_t) aiocbp;
-    req->sector_number = aiocbp->aio_offset / 512;
+    req->u.rw.sector_number = aiocbp->aio_offset / 512;
 
     for (j = 0; j < n; j++) {
-        req->seg[j].first_sect = 0;
-        req->seg[j].last_sect = PAGE_SIZE / 512 - 1;
+        req->u.rw.seg[j].first_sect = 0;
+        req->u.rw.seg[j].last_sect = PAGE_SIZE / 512 - 1;
     }
-    req->seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) / 512;
-    req->seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
+    req->u.rw.seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) / 512;
+    req->u.rw.seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
     for (j = 0; j < n; j++) {
 	uintptr_t data = start + j * PAGE_SIZE;
         if (!write) {
             /* Trigger CoW if needed */
-            *(char*)(data + (req->seg[j].first_sect << 9)) = 0;
+            *(char*)(data + (req->u.rw.seg[j].first_sect << 9)) = 0;
             barrier();
         }
-	aiocbp->gref[j] = req->seg[j].gref =
+	aiocbp->gref[j] = req->u.rw.seg[j].gref =
             gnttab_grant_access(dev->dom, virtual_to_mfn(data), write);
     }
 
@@ -432,7 +432,7 @@
     req->handle = dev->handle;
     req->id = id;
     /* Not needed anyway, but the backend will check it */
-    req->sector_number = 0;
+    req->u.rw.sector_number = 0;
     dev->ring.req_prod_pvt = i + 1;
     wmb();
     RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&dev->ring, notify);
diff -r 89116f28083f -r 7015d7407eae tools/blktap/drivers/tapdisk.c
--- a/tools/blktap/drivers/tapdisk.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap/drivers/tapdisk.c	Tue Dec 21 13:28:48 2010 +0000
@@ -528,10 +528,10 @@
 segment_start(blkif_request_t *req, int sidx)
 {
 	int i;
-	uint64_t start = req->sector_number;
+	uint64_t start = req->u.rw.sector_number;
 
 	for (i = 0; i < sidx; i++) 
-		start += (req->seg[i].last_sect - req->seg[i].first_sect + 1);
+		start += (req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1);
 
 	return start;
 }
@@ -600,13 +600,13 @@
 	struct disk_driver *parent = dd->next;
 	
 	seg_start = segment_start(req, sidx);
-	seg_end   = seg_start + req->seg[sidx].last_sect + 1;
+	seg_end   = seg_start + req->u.rw.seg[sidx].last_sect + 1;
 	
 	ASSERT(sector >= seg_start && sector + nr_secs <= seg_end);
 
 	page  = (char *)MMAP_VADDR(info->vstart, 
 				   (unsigned long)req->id, sidx);
-	page += (req->seg[sidx].first_sect << SECTOR_SHIFT);
+	page += (req->u.rw.seg[sidx].first_sect << SECTOR_SHIFT);
 	page += ((sector - seg_start) << SECTOR_SHIFT);
 
 	if (!parent) {
@@ -667,7 +667,7 @@
 			       req, sizeof(*req));
 			blkif->pending_list[idx].status = BLKIF_RSP_OKAY;
 			blkif->pending_list[idx].submitting = 1;
-			sector_nr = req->sector_number;
+			sector_nr = req->u.rw.sector_number;
 		}
 
 		if ((dd->flags & TD_RDONLY) && 
@@ -677,16 +677,16 @@
 		}
 
 		for (i = start_seg; i < req->nr_segments; i++) {
-			nsects = req->seg[i].last_sect - 
-				 req->seg[i].first_sect + 1;
+			nsects = req->u.rw.seg[i].last_sect - 
+				 req->u.rw.seg[i].first_sect + 1;
 	
-			if ((req->seg[i].last_sect >= page_size >> 9) ||
+			if ((req->u.rw.seg[i].last_sect >= page_size >> 9) ||
 			    (nsects <= 0))
 				continue;
 
 			page  = (char *)MMAP_VADDR(info->vstart, 
 						   (unsigned long)req->id, i);
-			page += (req->seg[i].first_sect << SECTOR_SHIFT);
+			page += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
 
 			if (sector_nr >= s->size) {
 				DPRINTF("Sector request failed:\n");
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-diff.c
--- a/tools/blktap2/drivers/tapdisk-diff.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-diff.c	Tue Dec 21 13:28:48 2010 +0000
@@ -363,13 +363,13 @@
 	breq                = &sreq->blkif_req;
 	breq->id            = idx;
 	breq->nr_segments   = r->blkif_req.nr_segments;
-	breq->sector_number = r->blkif_req.sector_number;
+	breq->u.rw.sector_number = r->blkif_req.u.rw.sector_number;
 	breq->operation     = BLKIF_OP_READ;
 
 	for (int i = 0; i < r->blkif_req.nr_segments; i++) {
-		struct blkif_request_segment *seg = breq->seg + i;
-		seg->first_sect = r->blkif_req.seg[i].first_sect;
-		seg->last_sect  = r->blkif_req.seg[i].last_sect;
+		struct blkif_request_segment *seg = breq->u.rw.seg + i;
+		seg->first_sect = r->blkif_req.u.rw.seg[i].first_sect;
+		seg->last_sect  = r->blkif_req.u.rw.seg[i].last_sect;
 	}
 	s->cur += sreq->secs;
 
@@ -426,12 +426,12 @@
 		breq                = &sreq->blkif_req;
 		breq->id            = idx;
 		breq->nr_segments   = 0;
-		breq->sector_number = sreq->sec;
+		breq->u.rw.sector_number = sreq->sec;
 		breq->operation     = BLKIF_OP_READ;
 
 		for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
 			uint32_t secs;
-			struct blkif_request_segment *seg = breq->seg + i;
+			struct blkif_request_segment *seg = breq->u.rw.seg + i;
 
 			secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
 			secs = MIN(((blk + 1) << SPB_SHIFT) - s->cur, secs);
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-image.c
--- a/tools/blktap2/drivers/tapdisk-image.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-image.c	Tue Dec 21 13:28:48 2010 +0000
@@ -148,15 +148,15 @@
 	psize = getpagesize();
 
 	for (i = 0; i < req->nr_segments; i++) {
-		nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
+		nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
 		
-		if (req->seg[i].last_sect >= psize >> 9 || nsects <= 0)
+		if (req->u.rw.seg[i].last_sect >= psize >> 9 || nsects <= 0)
 			goto fail;
 
 		total += nsects;
 	}
 
-	if (req->sector_number + nsects > info->size)
+	if (req->u.rw.sector_number + nsects > info->size)
 		goto fail;
 
 	return 0;
@@ -164,6 +164,6 @@
 fail:
 	ERR(-EINVAL, "bad request on %s (%s, %"PRIu64"): id: %"PRIu64": %d at %"PRIu64,
 	    image->name, (rdonly ? "ro" : "rw"), info->size, req->id,
-	    req->operation, req->sector_number + total);
+	    req->operation, req->u.rw.sector_number + total);
 	return -EINVAL;
 }
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-stream.c
--- a/tools/blktap2/drivers/tapdisk-stream.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-stream.c	Tue Dec 21 13:28:48 2010 +0000
@@ -293,12 +293,12 @@
 		breq                = &sreq->blkif_req;
 		breq->id            = idx;
 		breq->nr_segments   = 0;
-		breq->sector_number = sreq->sec;
+		breq->u.rw.sector_number = sreq->sec;
 		breq->operation     = BLKIF_OP_READ;
 
 		for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
 			uint32_t secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
-			struct blkif_request_segment *seg = breq->seg + i;
+			struct blkif_request_segment *seg = breq->u.rw.seg + i;
 
 			if (!secs)
 				break;
diff -r 89116f28083f -r 7015d7407eae tools/blktap2/drivers/tapdisk-vbd.c
--- a/tools/blktap2/drivers/tapdisk-vbd.c	Wed Dec 08 10:46:31 2010 +0000
+++ b/tools/blktap2/drivers/tapdisk-vbd.c	Tue Dec 21 13:28:48 2010 +0000
@@ -1066,7 +1066,7 @@
 	rsp->status = vreq->status;
 
 	DBG(TLOG_DBG, "writing req %d, sec 0x%08"PRIx64", res %d to ring\n",
-	    (int)tmp.id, tmp.sector_number, vreq->status);
+	    (int)tmp.id, tmp.u.rw.sector_number, vreq->status);
 
 	if (rsp->status != BLKIF_RSP_OKAY)
 		ERR(EIO, "returning BLKIF_RSP %d", rsp->status);
@@ -1181,10 +1181,10 @@
 tapdisk_vbd_breq_get_sector(blkif_request_t *breq, td_request_t treq)
 {
     int seg, nsects; 
-    uint64_t sector_nr = breq->sector_number; 
+    uint64_t sector_nr = breq->u.rw.sector_number; 
     
     for(seg=0; seg < treq.sidx; seg++) {
-        nsects = breq->seg[seg].last_sect - breq->seg[seg].first_sect + 1;
+        nsects = breq->u.rw.seg[seg].last_sect - breq->u.rw.seg[seg].first_sect + 1;
         sector_nr += nsects;
     }
 
@@ -1222,8 +1222,8 @@
 			uint16_t uid  = image->memshr_id;
 			blkif_request_t *breq = &vreq->req;
 			uint64_t sec  = tapdisk_vbd_breq_get_sector(breq, treq);
-			int secs = breq->seg[treq.sidx].last_sect -
-			    breq->seg[treq.sidx].first_sect + 1;
+			int secs = breq->u.rw.seg[treq.sidx].last_sect -
+			    breq->u.rw.seg[treq.sidx].first_sect + 1;
 
 			if (hnd != 0)
 				memshr_vbd_complete_ro_request(hnd, uid,
@@ -1288,7 +1288,7 @@
 			blkif_request_t *breq = &vreq->req;
         
 			ret = memshr_vbd_issue_ro_request(treq.buf,
-			      breq->seg[seg].gref,
+			      breq->u.rw.seg[seg].gref,
 			      parent->memshr_id,
 			      treq.sec,
 			      treq.secs,
@@ -1366,7 +1366,7 @@
 	req       = &vreq->req;
 	id        = req->id;
 	ring      = &vbd->ring;
-	sector_nr = req->sector_number;
+	sector_nr = req->u.rw.sector_number;
 	image     = tapdisk_vbd_first_image(vbd);
 
 	vreq->submitting = 1;
@@ -1385,10 +1385,10 @@
 		goto fail;
 
 	for (i = 0; i < req->nr_segments; i++) {
-		nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
+		nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
 		page   = (char *)MMAP_VADDR(ring->vstart, 
 					   (unsigned long)req->id, i);
-		page  += (req->seg[i].first_sect << SECTOR_SHIFT);
+		page  += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
 
 		treq.id             = id;
 		treq.sidx           = i;
@@ -1482,7 +1482,7 @@
 		vreq->status = BLKIF_RSP_OKAY;
 		DBG(TLOG_DBG, "retry #%d of req %"PRIu64", "
 		    "sec 0x%08"PRIx64", nr_segs: %d\n", vreq->num_retries,
-		    vreq->req.id, vreq->req.sector_number,
+		    vreq->req.id, vreq->req.u.rw.sector_number,
 		    vreq->req.nr_segments);
 
 		err = tapdisk_vbd_issue_request(vbd, vreq);
diff -r 89116f28083f -r 7015d7407eae xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Wed Dec 08 10:46:31 2010 +0000
+++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
@@ -98,13 +98,19 @@
     uint8_t     first_sect, last_sect;
 };
 
+struct blkif_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];
+};
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     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_request_rw rw;
+    } u;
 };
 typedef struct blkif_request blkif_request_t;

[PATCH 2/2 v2] Add trim operation to xen block devices

[Owen Smith]

Add Trim command interface

This patch adds the trim operation to the blkback ring protocol.

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. The specified sectors should be trimmed if the underlying 
block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP should 
be returned. More information about trim operations:
http://t13.org/Documents/UploadedDocuments/docs2008/
       e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

Signed-of-by: Owen Smith <owen.smith@citrix.com>

# HG changeset patch
# User Owen Smith <owen.smith@citrix.com>
# Date 1292938216 0
# Node ID 6be1a23977b82e44eba80feeca3fb77acf6bb2bb
# Parent  7015d7407eae8694ce1cc92e792b2087f109f6cb
imported patch add-trim

diff -r 7015d7407eae -r 6be1a23977b8 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Tue Dec 21 13:28:48 2010 +0000
+++ b/xen/include/public/io/blkif.h	Tue Dec 21 13:30:16 2010 +0000
@@ -76,6 +76,17 @@
  * "feature-flush-cache" node!
  */
 #define BLKIF_OP_FLUSH_DISKCACHE   3
+/*
+ * 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              5
 
 /*
  * Maximum scatter/gather segments per request.
@@ -103,6 +114,11 @@
     struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
 
+struct blkif_request_trim {
+    blkif_sector_t sector_number;/* start sector idx on disk   */
+    uint64_t nr_sectors;
+};
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     uint8_t        nr_segments;  /* number of segments                   */
@@ -110,6 +126,7 @@
     uint64_t       id;           /* private guest value, echoed in resp  */
     union {
     	struct blkif_request_rw rw;
+    	struct blkif_request_trim trim;
     } u;
 };
 typedef struct blkif_request blkif_request_t;