xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* blkif: drop struct blkif_request_segment_aligned
@ 2014-02-04  8:15 Jan Beulich
  2014-02-04  8:25 ` Roger Pau Monné
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2014-02-04  8:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Keir Fraser, Roger Pau Monne

[-- Attachment #1: Type: text/plain, Size: 2742 bytes --]

Commit 5148b7b5 ("blkif: add indirect descriptors interface to public
headers") added this without really explaining why it is needed: The
structure is identical to struct blkif_request_segment apart from the
padding field not being given a name in the pre-existing type. Their
size and alignment - which are what is relevant - are identical as long
as __alignof__(uint32_t) == 4 (which I think we rely upon in various
other places, so we can take as given).

Also correct a few minor glitches in the description, including for it
to no longer assume PAGE_SIZE == 4096.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -487,13 +487,13 @@
  * it's less than the number provided by the backend. The indirect_grefs field
  * in blkif_request_indirect should be filled by the frontend with the
  * grant references of the pages that are holding the indirect segments.
- * This pages are filled with an array of blkif_request_segment_aligned
- * that hold the information about the segments. The number of indirect
- * pages to use is determined by the maximum number of segments
- * an indirect request contains. Every indirect page can contain a maximum
- * of 512 segments (PAGE_SIZE/sizeof(blkif_request_segment_aligned)),
- * so to calculate the number of indirect pages to use we have to do
- * ceil(indirect_segments/512).
+ * These pages are filled with an array of blkif_request_segment that hold the
+ * information about the segments. The number of indirect pages to use is
+ * determined by the number of segments an indirect request contains. Every
+ * indirect page can contain a maximum of
+ * (PAGE_SIZE / sizeof(struct blkif_request_segment)) segments, so to
+ * calculate the number of indirect pages to use we have to do
+ * ceil(indirect_segments / (PAGE_SIZE / sizeof(struct blkif_request_segment))).
  *
  * If a backend does not recognize BLKIF_OP_INDIRECT, it should *not*
  * create the "feature-max-indirect-segments" node!
@@ -569,14 +569,6 @@ struct blkif_request_indirect {
 };
 typedef struct blkif_request_indirect blkif_request_indirect_t;
 
-struct blkif_request_segment_aligned {
-    grant_ref_t gref;            /* reference to I/O buffer frame        */
-    /* @first_sect: first sector in frame to transfer (inclusive).   */
-    /* @last_sect: last sector in frame to transfer (inclusive).     */
-    uint8_t     first_sect, last_sect;
-    uint16_t    _pad; /* padding to make it 8 bytes, so it's cache-aligned */
-};
-
 struct blkif_response {
     uint64_t        id;              /* copied from request */
     uint8_t         operation;       /* copied from request */




[-- Attachment #2: blkif-drop-segment-aligned.patch --]
[-- Type: text/plain, Size: 2788 bytes --]

blkif: drop struct blkif_request_segment_aligned

Commit 5148b7b5 ("blkif: add indirect descriptors interface to public
headers") added this without really explaining why it is needed: The
structure is identical to struct blkif_request_segment apart from the
padding field not being given a name in the pre-existing type. Their
size and alignment - which are what is relevant - are identical as long
as __alignof__(uint32_t) == 4 (which I think we rely upon in various
other places, so we can take as given).

Also correct a few minor glitches in the description, including for it
to no longer assume PAGE_SIZE == 4096.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -487,13 +487,13 @@
  * it's less than the number provided by the backend. The indirect_grefs field
  * in blkif_request_indirect should be filled by the frontend with the
  * grant references of the pages that are holding the indirect segments.
- * This pages are filled with an array of blkif_request_segment_aligned
- * that hold the information about the segments. The number of indirect
- * pages to use is determined by the maximum number of segments
- * an indirect request contains. Every indirect page can contain a maximum
- * of 512 segments (PAGE_SIZE/sizeof(blkif_request_segment_aligned)),
- * so to calculate the number of indirect pages to use we have to do
- * ceil(indirect_segments/512).
+ * These pages are filled with an array of blkif_request_segment that hold the
+ * information about the segments. The number of indirect pages to use is
+ * determined by the number of segments an indirect request contains. Every
+ * indirect page can contain a maximum of
+ * (PAGE_SIZE / sizeof(struct blkif_request_segment)) segments, so to
+ * calculate the number of indirect pages to use we have to do
+ * ceil(indirect_segments / (PAGE_SIZE / sizeof(struct blkif_request_segment))).
  *
  * If a backend does not recognize BLKIF_OP_INDIRECT, it should *not*
  * create the "feature-max-indirect-segments" node!
@@ -569,14 +569,6 @@ struct blkif_request_indirect {
 };
 typedef struct blkif_request_indirect blkif_request_indirect_t;
 
-struct blkif_request_segment_aligned {
-    grant_ref_t gref;            /* reference to I/O buffer frame        */
-    /* @first_sect: first sector in frame to transfer (inclusive).   */
-    /* @last_sect: last sector in frame to transfer (inclusive).     */
-    uint8_t     first_sect, last_sect;
-    uint16_t    _pad; /* padding to make it 8 bytes, so it's cache-aligned */
-};
-
 struct blkif_response {
     uint64_t        id;              /* copied from request */
     uint8_t         operation;       /* copied from request */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: blkif: drop struct blkif_request_segment_aligned
  2014-02-04  8:15 blkif: drop struct blkif_request_segment_aligned Jan Beulich
@ 2014-02-04  8:25 ` Roger Pau Monné
  0 siblings, 0 replies; 2+ messages in thread
From: Roger Pau Monné @ 2014-02-04  8:25 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Keir Fraser

On 04/02/14 09:15, Jan Beulich wrote:
> Commit 5148b7b5 ("blkif: add indirect descriptors interface to public
> headers") added this without really explaining why it is needed: The
> structure is identical to struct blkif_request_segment apart from the
> padding field not being given a name in the pre-existing type. Their
> size and alignment - which are what is relevant - are identical as long
> as __alignof__(uint32_t) == 4 (which I think we rely upon in various
> other places, so we can take as given).
>
> Also correct a few minor glitches in the description, including for it
> to no longer assume PAGE_SIZE == 4096.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Thanks for the patch.

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-02-04  8:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-04  8:15 blkif: drop struct blkif_request_segment_aligned Jan Beulich
2014-02-04  8:25 ` Roger Pau Monné

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).