Skip to contentSkip to conversation

virtio: fix size computation according to the definition of struct vring_used in vring_size

PATCH Applied Wang Sheng-Hui 4 participants 19 messages Aug 27, 2011 – Aug 30, 2011 virtio core

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 virtio: fix size computation according to the definition of struct vring_used in vring_size

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
Signature · 3 lines
1.7.1

crossover@crossover-plinux:~/dev/patchwork$ 
#1 lore raw
Quoted 15 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +0 -0
index 4a32cb6..fcda152 100644

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 8 lines
include/linux/virtio_ring.h +1 -1
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

#2 lore raw
In reply to Wang Sheng-Hui
Quoted 15 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +0 -0
index 4a32cb6..fcda152 100644

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 8 lines
include/linux/virtio_ring.h +1 -1
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

#3 lore raw
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?
_______________________________________________
Virtualization mailing list [email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

#4 lore raw
In reply to Wanlong Gao
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

#5 lore raw
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 7 lines
1.7.1


_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
#6 lore raw
In reply to Wang Sheng-Hui
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1
#7 lore raw
Quoted 44 lines · Wang Sheng-Hui
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

Quoted 8 lines

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

Is this a bugfix or just a cleanup?
Alignment makes us get the same values with and without, right?

Quoted 28 lines
---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1
#8 lore raw
Quoted 49 lines · Michael S. Tsirkin
Quoted 48 lines · Wang Sheng-Hui
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Quoted 7 lines
Quoted 4 lines

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

Is this a bugfix or just a cleanup?
Alignment makes us get the same values with and without, right?

It's just a cleanup.
For normal case, the value 16 is too small compared with the alignment, I think.

Quoted 19 lines
Quoted 19 lines
---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +2 -2
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
Quoted 10 lines
Quoted 10 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

#9 lore raw
In reply to Michael S. Tsirkin
Quoted 49 lines · Michael S. Tsirkin
Quoted 48 lines · Wang Sheng-Hui
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Quoted 7 lines
Quoted 4 lines

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

Is this a bugfix or just a cleanup?
Alignment makes us get the same values with and without, right?

It's just a cleanup.
For normal case, the value 16 is too small compared with the alignment, I think.

Quoted 29 lines
Quoted 29 lines
---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1
#10 lore raw
In reply to Wang Sheng-Hui
Quoted 44 lines · Wang Sheng-Hui
Quoted 34 lines · Wang Sheng-Hui
Quoted 23 lines · Wanlong Gao
Quoted 22 lines · Wang Sheng-Hui

The patch is against 3.1-rc3.

struct vring_used has two __u16 fields plus array of struct vring_used_elem. Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.

---
 include/linux/virtio_ring.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/virtio_ring.h +1 -1
index 4a32cb6..fcda152 100644
@@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
 	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
 		 + align - 1) & ~(align - 1))
-		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
+		+ sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
 }
 
Quoted 9 lines

include/linux/virtio_ring.h static inline unsigned vring_size(unsigned int num, unsigned long align) · line 149

Quoted 1 line
include/linux/virtio_ring.h +0 -0
 /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */

Hi:
I'm not deep into it, but I think you can see this:
http://marc.info/?l=git-commits-head&m=130687915816130&w=2

Thanks
-Wanlong Gao

Quoted 5 lines

Thanks.

I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right?

New patch generated. Please check it. Thanks,

Quoted 8 lines

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

Is this a bugfix or just a cleanup?
Alignment makes us get the same values with and without, right?

Quoted 28 lines
---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

#11 lore raw
In reply to Wang Sheng-Hui
Quoted 6 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 8 lines
include/linux/virtio_ring.h +2 -2
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 146

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

#12 lore raw
In reply to Wang Sheng-Hui
Quoted 6 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 8 lines
include/linux/virtio_ring.h +2 -2
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 146

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.

#13 lore raw
Quoted 10 lines · Rusty Russell
Quoted 9 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
Quoted 8 lines

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 5 lines
include/linux/virtio_ring.h +2 -2
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

Yes.

Quoted 18 lines

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 146

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

Quoted 5 lines

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

#14 lore raw
In reply to Rusty Russell
Quoted 10 lines · Rusty Russell
Quoted 9 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
Quoted 8 lines

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 5 lines
include/linux/virtio_ring.h +2 -2
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

Yes.

Quoted 23 lines

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 146

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.

#15 lore raw
In reply to Rusty Russell
Quoted 36 lines · Rusty Russell
Quoted 6 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 8 lines
include/linux/virtio_ring.h +2 -2
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 145

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

Quoted 5 lines

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.

Patch regerated to use sizeof(__u16) instead of 16 in vring_init. Please check it.

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..2a731bd 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + sizeof(__16)
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 7 lines
1.7.1


_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
#16 lore raw
In reply to Rusty Russell
Quoted 36 lines · Rusty Russell
Quoted 6 lines · Wang Sheng-Hui

On 2011年08月27日 17:34, Wang Sheng-Hui wrote:

include/linux/virtio_ring.h +0 -0
index 4a32cb6..300af76 100644

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 140

Quoted 8 lines
include/linux/virtio_ring.h +2 -2
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
+		+ align-1) & ~(align - 1));
 }

This + 16 should be + sizeof(__u16), right? It's just the used_event_idx which has been added:

* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]

include/linux/virtio_ring.h static inline void vring_init(struct vring *vr, unsigned int num, void *p, · line 145

Quoted 6 lines
include/linux/virtio_ring.h +1 -1
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;

This is correct.

Quoted 5 lines

And, yes, since align is always 4096, it's currently just a cleanup, but it makes things much less confusing!

Thanks,
Rusty.

Patch regerated to use sizeof(__u16) instead of 16 in vring_init. Please check it.

[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx

The patch is against 3.1-rc3.

Based on the layout description in the comments, take account of the *_event_idx in functions vring_init and vring_size.

---
 include/linux/virtio_ring.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/virtio_ring.h +3 -3
index 4a32cb6..2a731bd 100644
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
 	vr->num = num;
 	vr->desc = p;
 	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
-			    & ~(align - 1));
+	vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + sizeof(__16)
+		+ align-1) & ~(align - 1));
 }
 
 static inline unsigned vring_size(unsigned int num, unsigned long align)
 {
-	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+	return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
 		 + align - 1) & ~(align - 1))
 		+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
 }
Signature · 1 line
1.7.1
#17 lore raw
Quoted 2 lines · Wang Sheng-Hui

Patch regerated to use sizeof(__u16) instead of 16 in vring_init. Please check it.

Applied.

Thanks,
Rusty.