thr3ads.net - Xen devel - [Xen-devel] [PATCH 000] blkif: Introduce trim interface [Jan 2011]

If this information is useful, please help other people find it:
Share via:

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

[Xen-devel] [PATCH 000] blkif: Introduce trim interface

Trim operation is a request for the underlying block device to mark
extents to be erased. This is a stub patch that responds to trim 
operations with BLKIF_RSP_EOPNOTSUPP and is intended to specify
the blkif ring interface.
More information about trim operations at;
http://t13.org/Documents/UploadedDocuments/docs2008/
    e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

This patch set touches 3 repositories with similar changes to each.

xen-unstable.hg
1/2 xen-unstable/blkif: Moves the read/write/barrier request specific 
      fields into a union.
2/2 xen-unstable/blkif: Defines the operation code and structure of a
      trim operation.

xen.git
1/2 pvops/blkif: Moves the read/write/barrier request specific fields 
      into a union.
2/2 pvops/blkif: Defines the operation code and structure of a trim 
      operation. Responds to trim operations with BLKIF_RSP_EOPNOTSUPP.

qemu-xen.git
1/1 qemu-xen/blkif: Moves the read/write/barrier request specific 
      fields into a union. Needed to compile qemu-xen as part of
      xen-unstable''s build.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 0/2] xen-unstable/blkif: Introduce trim interface

From: Owen Smith <owen.smith@citrix.com>

Patches xen-unstable.hg

This patch reorganises the blkif ring structure and extends it to add
a very basic implementation of trim operations.

Details on trim;
http://t13.org/Documents/UploadedDocuments/docs2008/
    e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

1/2 : Moves the read/write/barrier specific fields into a union.
2/2 : Adds the trim operation code and trim fields.
      
This patch series will require the related patch to qemu-xen.git to 
compile.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

From: Owen Smith <owen.smith@citrix.com>

# HG 
changeset patch
# User root@yogurt.uk.xensource.com
# Date 1294919321 0
# Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
# Parent  7b4c82f07281ad9c48b652e2a305a7be607c5283
blkif: Move read/write/barrier specific fields into a union

Patches xen-unstable.hg

Modifies the blkif ring interface by placing the request specific
fields into a union in order to support additional operation types.

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

diff -r 7b4c82f07281 -r 428420495fd3 extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/extras/mini-os/blkfront.c	Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap/drivers/tapdisk.c
--- a/tools/blktap/drivers/tapdisk.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/tools/blktap/drivers/tapdisk.c	Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-diff.c
--- a/tools/blktap2/drivers/tapdisk-diff.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/tools/blktap2/drivers/tapdisk-diff.c	Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-image.c
--- a/tools/blktap2/drivers/tapdisk-image.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/tools/blktap2/drivers/tapdisk-image.c	Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-stream.c
--- a/tools/blktap2/drivers/tapdisk-stream.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/tools/blktap2/drivers/tapdisk-stream.c	Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-vbd.c
--- a/tools/blktap2/drivers/tapdisk-vbd.c	Wed Jan 05 23:54:15 2011 +0000
+++ b/tools/blktap2/drivers/tapdisk-vbd.c	Thu Jan 13 11:48:41 2011 +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;
     }
 
@@ -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 7b4c82f07281 -r 428420495fd3 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Wed Jan 05 23:54:15 2011 +0000
+++ b/xen/include/public/io/blkif.h	Thu Jan 13 11:48:41 2011 +0000
@@ -103,7 +103,22 @@
     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  */
+    union {
+        struct blkif_request_rw     rw;
+    } u;
+};
+
+struct blkif_request_old {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     uint8_t        nr_segments;  /* number of segments                   */
     blkif_vdev_t   handle;       /* only for read/write requests         */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 2/2] xen-unstable/blkif: Add trim operation interface

From: Owen Smith <owen.smith@citrix.com>

# HG 
changeset patch
# User root@yogurt.uk.xensource.com
# Date 1294921282 0
# Node ID ab006de92c098a54d9e029e885f91d5c3c62c3c0
# Parent  428420495fd3cfda164e7b355b0ffcc0845af2e4
blkif: Add trim operation interface

Patches xen-unstable.hg

Trim operation is a request for the underlying block device to mark
extents to be erased. Add the operation code and ring data structure
to the public header file.

Trim operations 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 at;
http://t13.org/Documents/UploadedDocuments/docs2008/
    e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc

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

diff -r 428420495fd3 -r ab006de92c09 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Thu Jan 13 11:48:41 2011 +0000
+++ b/xen/include/public/io/blkif.h	Thu Jan 13 12:21:22 2011 +0000
@@ -81,6 +81,17 @@
  * contained within the request. Reserved for that purpose.
  */
 #define BLKIF_OP_RESERVED_1        4
+/*
+ * 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.
@@ -108,6 +119,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;   /* number of sectors to trim */
+};
+
 struct blkif_request {
     uint8_t        operation;    /* BLKIF_OP_???                         */
     uint8_t        nr_segments;  /* number of segments                   */
@@ -115,6 +131,7 @@
     uint64_t       id;           /* private guest value, echoed in resp  */
     union {
         struct blkif_request_rw     rw;
+        struct blkif_request_trim   trim;
     } u;
 };
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 1/1] qemu-xen/blkif: Move read/write/barrier specific fields into a union

From: Owen Smith <owen.smith@citrix.com>

Patches qemu-xen.git, on branch dummy as part of building xen-unstable.hg

Modifies the blkif ring interface by placing the request specific
fields into a union in order to support additional operation types.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
---
 hw/xen_blkif.h  |    8 ++++----
 hw/xen_blktap.c |   10 +++++-----
 hw/xen_disk.c   |   14 +++++++-------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/xen_blkif.h b/hw/xen_blkif.h
index ca3a65b..de3e103 100644
--- a/hw/xen_blkif.h
+++ b/hw/xen_blkif.h
@@ -78,11 +78,11 @@ static inline void blkif_get_x86_32_req(blkif_request_t
*dst, blkif_x86_32_reque
 	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;
 	if (n > src->nr_segments)
 		n = src->nr_segments;
 	for (i = 0; i < n; i++)
-		dst->seg[i] = src->seg[i];
+		dst->u.rw.seg[i] = src->seg[i];
 }
 
 static inline void blkif_get_x86_64_req(blkif_request_t *dst,
blkif_x86_64_request_t *src)
@@ -93,11 +93,11 @@ static inline void blkif_get_x86_64_req(blkif_request_t
*dst, blkif_x86_64_reque
 	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;
 	if (n > src->nr_segments)
 		n = src->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/hw/xen_blktap.c b/hw/xen_blktap.c
index 24d10a3..ab3210a 100644
--- a/hw/xen_blktap.c
+++ b/hw/xen_blktap.c
@@ -383,7 +383,7 @@ static void handle_blktap_iomsg(void* private)
 		memcpy(&blkif->pending_list[idx].req, 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;
 
 		/* Don''t allow writes on readonly devices */
 		if ((s->flags & TD_RDONLY) && 
@@ -393,16 +393,16 @@ static void handle_blktap_iomsg(void* private)
 		}
 
 		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  = (uint8_t*) 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 --git a/hw/xen_disk.c b/hw/xen_disk.c
index 218f654..b427409 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -176,7 +176,7 @@ static int ioreq_parse(struct ioreq *ioreq)
     xen_be_printf(&blkdev->xendev, 3,
 		  "op %d, nr %d, handle %d, id %" PRId64 ", sector %"
PRId64 "\n",
 		  ioreq->req.operation, ioreq->req.nr_segments,
-		  ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
+		  ioreq->req.handle, ioreq->req.id, ioreq->req.u.rw.sector_number);
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
 	ioreq->prot = PROT_WRITE; /* to memory */
@@ -205,26 +205,26 @@ static int ioreq_parse(struct ioreq *ioreq)
         goto err;
     }
 
-    ioreq->start = ioreq->req.sector_number * blkdev->file_blk;
+    ioreq->start = ioreq->req.u.rw.sector_number * blkdev->file_blk;
     for (i = 0; i < ioreq->req.nr_segments; i++) {
 	if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
 	    xen_be_printf(&blkdev->xendev, 0, "error: nr_segments too
big\n");
 	    goto err;
 	}
-	if (ioreq->req.seg[i].first_sect > ioreq->req.seg[i].last_sect) {
+	if (ioreq->req.u.rw.seg[i].first_sect >
ioreq->req.u.rw.seg[i].last_sect) {
 	    xen_be_printf(&blkdev->xendev, 0, "error: first > last
sector\n");
 	    goto err;
 	}
-	if (ioreq->req.seg[i].last_sect * BLOCK_SIZE >= XC_PAGE_SIZE) {
+	if (ioreq->req.u.rw.seg[i].last_sect * BLOCK_SIZE >= XC_PAGE_SIZE) {
 	    xen_be_printf(&blkdev->xendev, 0, "error: page
crossing\n");
 	    goto err;
 	}
 
 	ioreq->domids[i] = blkdev->xendev.dom;
-	ioreq->refs[i]   = ioreq->req.seg[i].gref;
+	ioreq->refs[i]   = ioreq->req.u.rw.seg[i].gref;
 
-	mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
-	len = (ioreq->req.seg[i].last_sect - ioreq->req.seg[i].first_sect + 1) *
blkdev->file_blk;
+	mem = ioreq->req.u.rw.seg[i].first_sect * blkdev->file_blk;
+	len = (ioreq->req.u.rw.seg[i].last_sect -
ioreq->req.u.rw.seg[i].first_sect + 1) * blkdev->file_blk;
         qemu_iovec_add(&ioreq->v, (void*)mem, len);
     }
     if (ioreq->start + ioreq->v.size > blkdev->file_size) {
-- 
1.5.6.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 0/2] pvops/blkif: Introduce trim interface

From: Owen Smith <owen.smith@citrix.com>

Patches xen.git, branch xen/next-2.6.32

Moves the read/write/barrier request specific fields into a
union and implements a basic backend failure response for 
trim operations.

Details on trim;
http://t13.org/Documents/UploadedDocuments/docs2008/
    e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc
    
1/2 : Moves read/write/barrier request specific fields into a union.
2/2 : Adds the trim operation code, trim structure and blkback response.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 1/2] pvops/blkif: Move read/write/barrier specific fields into a union

From: Owen Smith <owen.smith@citrix.com>

Patches xen.git, on branch xen/next-2.6.32

Modifies the blkif ring interface by placing the request specific
fields into a union in order to support additional operation types.

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 9679ffa..9ac31d2 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

<owen.smith@citrix.com>

2011-Jan-14 16:44 UTC

head link

[Xen-devel] [PATCH 2/2] pvops/blkif: Add trim operation interface

From: Owen Smith <owen.smith@citrix.com>

Patches xen.git, on branch xen/next-2.6.32

Trim operation is a request for the underlying block device to mark
extents to be erased. Add the operation code and ring data structure
to the interface, and a stub response to the blkback device.

Trim operations 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 at;
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..2ae8840 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Christoph Egger

2011-Jan-14 16:55 UTC

head link

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

On Friday 14 January 2011 17:44:22 owen.smith@citrix.com
wrote:> From: Owen Smith <owen.smith@citrix.com>
>
> # HG
> changeset patch
> # User root@yogurt.uk.xensource.com
> # Date 1294919321 0
> # Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
> # Parent  7b4c82f07281ad9c48b652e2a305a7be607c5283
> blkif: Move read/write/barrier specific fields into a union
>
> Patches xen-unstable.hg
>
> Modifies the blkif ring interface by placing the request specific
> fields into a union in order to support additional operation types.
>
> Signed-off-by: Owen Smith <owen.smith@citrix.com>
When a blkif interface change is acceptable can you also
allow to deal with 64kb blocksize, please?
This fixes a performance problem with *BSD guests then.

Thanks,
Christoph

>
> diff -r 7b4c82f07281 -r 428420495fd3 extras/mini-os/blkfront.c
> --- a/extras/mini-os/blkfront.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/extras/mini-os/blkfront.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap/drivers/tapdisk.c
> --- a/tools/blktap/drivers/tapdisk.c    Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap/drivers/tapdisk.c    Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-diff.c
> --- a/tools/blktap2/drivers/tapdisk-diff.c      Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-diff.c      Thu Jan 13 11:48:41
> 2011 +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 7b4c82f07281 -r 428420495fd3
> tools/blktap2/drivers/tapdisk-image.c ---
> a/tools/blktap2/drivers/tapdisk-image.c     Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-image.c     Thu Jan 13 11:48:41 2011
> +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-stream.c
> --- a/tools/blktap2/drivers/tapdisk-stream.c    Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-stream.c    Thu Jan 13 11:48:41
> 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-vbd.c
> --- a/tools/blktap2/drivers/tapdisk-vbd.c       Wed Jan 05 23:54:15 2011
> +0000 +++ b/tools/blktap2/drivers/tapdisk-vbd.c       Thu Jan 13 11:48:41
> 2011 +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;
>      }
>
> @@ -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 7b4c82f07281 -r 428420495fd3 xen/include/public/io/blkif.h
> --- a/xen/include/public/io/blkif.h     Wed Jan 05 23:54:15 2011 +0000
> +++ b/xen/include/public/io/blkif.h     Thu Jan 13 11:48:41 2011 +0000
> @@ -103,7 +103,22 @@
>      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  */ +    union {
> +        struct blkif_request_rw     rw;
> +    } u;
> +};
> +
> +struct blkif_request_old {
>      uint8_t        operation;    /* BLKIF_OP_???                        
> */ uint8_t        nr_segments;  /* number of segments                   */
> blkif_vdev_t   handle;       /* only for read/write requests         */
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2011-Jan-14 17:05 UTC

head link

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

Redefining the blkif_req struct naming seems an unnecessary pain. Why not
define a separate ''struct blkif_trim_request'' that starts with
the uint8_t
BLKIF_OP_TRIM and thereafter is defined exactly how you like? Then drivers
can cast a request pointer to your new type when reading/writing a trim
request, and no existing code needs to be changed. If all this does is
introduce new definitions into xen-unstable, rather than changing existing
ones, we could still potentially slip it in for 4.1.0 as an obviously safe
patch.

 -- Keir

On 14/01/2011 16:44, "owen.smith@citrix.com"
<owen.smith@citrix.com> wrote:
> From: Owen Smith <owen.smith@citrix.com>
> 
> # HG 
> changeset patch
> # User root@yogurt.uk.xensource.com
> # Date 1294919321 0
> # Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
> # Parent  7b4c82f07281ad9c48b652e2a305a7be607c5283
> blkif: Move read/write/barrier specific fields into a union
> 
> Patches xen-unstable.hg
> 
> Modifies the blkif ring interface by placing the request specific
> fields into a union in order to support additional operation types.
> 
> Signed-off-by: Owen Smith <owen.smith@citrix.com>
> 
> diff -r 7b4c82f07281 -r 428420495fd3 extras/mini-os/blkfront.c
> --- a/extras/mini-os/blkfront.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/extras/mini-os/blkfront.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap/drivers/tapdisk.c
> --- a/tools/blktap/drivers/tapdisk.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap/drivers/tapdisk.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-diff.c
> --- a/tools/blktap2/drivers/tapdisk-diff.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-diff.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-image.c
> --- a/tools/blktap2/drivers/tapdisk-image.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-image.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-stream.c
> --- a/tools/blktap2/drivers/tapdisk-stream.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-stream.c Thu Jan 13 11:48:41 2011 +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 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-vbd.c
> --- a/tools/blktap2/drivers/tapdisk-vbd.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-vbd.c Thu Jan 13 11:48:41 2011 +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;
>      }
>  
> @@ -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 7b4c82f07281 -r 428420495fd3 xen/include/public/io/blkif.h
> --- a/xen/include/public/io/blkif.h Wed Jan 05 23:54:15 2011 +0000
> +++ b/xen/include/public/io/blkif.h Thu Jan 13 11:48:41 2011 +0000
> @@ -103,7 +103,22 @@
>      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 
*/
> +    union {
> +        struct blkif_request_rw     rw;
> +    } u;
> +};
> +
> +struct blkif_request_old {
>      uint8_t        operation;    /* BLKIF_OP_???                        
*/
>      uint8_t        nr_segments;  /* number of segments                  
*/
>      blkif_vdev_t   handle;       /* only for read/write requests        
*/
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser

2011-Jan-14 17:15 UTC

head link

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

On 14/01/2011 16:55, "Christoph Egger" <Christoph.Egger@amd.com>
wrote:
>> Patches xen-unstable.hg
>> 
>> Modifies the blkif ring interface by placing the request specific
>> fields into a union in order to support additional operation types.
>> 
>> Signed-off-by: Owen Smith <owen.smith@citrix.com>
> 
> When a blkif interface change is acceptable can you also
> allow to deal with 64kb blocksize, please?
> This fixes a performance problem with *BSD guests then.
The binary interface is not being changed here, it''s source-level
renaming
only. And thus I hope we can avoid it altogether.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Owen Smith

2011-Jan-17 17:04 UTC

head link

RE: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

Attached is a patch that does not require any union of existing structures.
>From the previous patch series,[PATCH 1/2] xen-unstable/blkif: is obsolete 
[PATCH 2/2] xen-unstable/blkif: is replaced by the attached patch.

The attached version 2 patch only adds definitions to the public header as
suggested.

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser
> Sent: 14 January 2011 17:05
> To: Owen Smith; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move
> read/write/barrier specific fields into a union
> 
> Redefining the blkif_req struct naming seems an unnecessary pain. Why
> not
> define a separate ''struct blkif_trim_request'' that starts
with the
> uint8_t
> BLKIF_OP_TRIM and thereafter is defined exactly how you like? Then
> drivers
> can cast a request pointer to your new type when reading/writing a trim
> request, and no existing code needs to be changed. If all this does is
> introduce new definitions into xen-unstable, rather than changing
> existing
> ones, we could still potentially slip it in for 4.1.0 as an obviously
> safe
> patch.
> 
>  -- Keir
> 
> On 14/01/2011 16:44, "owen.smith@citrix.com"
<owen.smith@citrix.com>
> wrote:
> 
> > From: Owen Smith <owen.smith@citrix.com>
> >
> > # HG
> > changeset patch
> > # User root@yogurt.uk.xensource.com
> > # Date 1294919321 0
> > # Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
> > # Parent  7b4c82f07281ad9c48b652e2a305a7be607c5283
> > blkif: Move read/write/barrier specific fields into a union
> >
> > Patches xen-unstable.hg
> >
> > Modifies the blkif ring interface by placing the request specific
> > fields into a union in order to support additional operation types.
> >
> > Signed-off-by: Owen Smith <owen.smith@citrix.com>
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Jackson

2011-Jan-17 18:03 UTC

head link

Re: [Xen-devel] [PATCH 1/1] qemu-xen/blkif: Move read/write/barrier specific fields into a union

owen.smith@citrix.com writes ("[Xen-devel] [PATCH 1/1] qemu-xen/blkif: Move
read/write/barrier specific fields into a union"):> From: Owen Smith <owen.smith@citrix.com>
> 
> Patches qemu-xen.git, on branch dummy as part of building xen-unstable.hg
> 
> Modifies the blkif ring interface by placing the request specific
> fields into a union in order to support additional operation types.
Thanks but I think this ought to wait for after the 4.1 release.
We''re in a pretty solid feature freeze at the moment.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Konrad Rzeszutek Wilk

2011-Jan-21 15:14 UTC

head link

Re: [Xen-devel] [PATCH 2/2] pvops/blkif: Add trim operation interface

On Fri, Jan 14, 2011 at 04:44:27PM +0000, owen.smith@citrix.com
wrote:> From: Owen Smith <owen.smith@citrix.com>
> 
> Patches xen.git, on branch xen/next-2.6.32
> 
> Trim operation is a request for the underlying block device to mark
> extents to be erased. Add the operation code and ring data structure
> to the interface, and a stub response to the blkback device.
> 
> Trim operations 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
Ok, so this code just returns EOPNOTSUPP. Is there a forthcomming patch for
supporting the trim operation?
> be returned. More information about trim operations at;
> 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..2ae8840 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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Jan 2011 - [PATCH 000] blkif: Introduce trim interface

[Xen-devel] [PATCH 000] blkif: Introduce trim interface

[Xen-devel] [PATCH 0/2] xen-unstable/blkif: Introduce trim interface

[Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

[Xen-devel] [PATCH 2/2] xen-unstable/blkif: Add trim operation interface

[Xen-devel] [PATCH 1/1] qemu-xen/blkif: Move read/write/barrier specific fields into a union

[Xen-devel] [PATCH 0/2] pvops/blkif: Introduce trim interface

[Xen-devel] [PATCH 1/2] pvops/blkif: Move read/write/barrier specific fields into a union

[Xen-devel] [PATCH 2/2] pvops/blkif: Add trim operation interface

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

Re: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

RE: [Xen-devel] [PATCH 1/2] xen-unstable/blkif: Move read/write/barrier specific fields into a union

Re: [Xen-devel] [PATCH 1/1] qemu-xen/blkif: Move read/write/barrier specific fields into a union

Re: [Xen-devel] [PATCH 2/2] pvops/blkif: Add trim operation interface