* [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size @ 2011-08-27 1:07 Wang Sheng-Hui 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 2:49 ` Wanlong Gao 0 siblings, 2 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 1:07 UTC (permalink / raw) To: rusty, mst, virtualization, linux-kernel 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..fcda152 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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 */ -- 1.7.1 crossover@crossover-plinux:~/dev/patchwork$ ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 1:07 [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size Wang Sheng-Hui @ 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 2:49 ` Wanlong Gao 1 sibling, 0 replies; 20+ messages in thread From: Wanlong Gao @ 2011-08-27 2:49 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: virtualization, linux-kernel, mst On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: > 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. > > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > --- > include/linux/virtio_ring.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..fcda152 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 1:07 [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size Wang Sheng-Hui 2011-08-27 2:49 ` Wanlong Gao @ 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 9:34 ` Wang Sheng-Hui 2011-08-27 9:34 ` Wang Sheng-Hui 1 sibling, 2 replies; 20+ messages in thread From: Wanlong Gao @ 2011-08-27 2:49 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: rusty, mst, virtualization, linux-kernel On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: > 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. > > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > --- > include/linux/virtio_ring.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..fcda152 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 2:49 ` Wanlong Gao @ 2011-08-27 9:34 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-27 9:34 ` Wang Sheng-Hui 1 sibling, 2 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 9:34 UTC (permalink / raw) To: wanlong.gao; +Cc: rusty, mst, virtualization, linux-kernel On 2011年08月27日 10:49, Wanlong Gao wrote: > On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >> 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. >> >> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >> --- >> include/linux/virtio_ring.h | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..fcda152 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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 > Thanks. I checked the comments, and think we should patch vring_init and vring_size according to the layout description in the comments. Right? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:34 ` Wang Sheng-Hui @ 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui 1 sibling, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 9:52 UTC (permalink / raw) To: wanlong.gao; +Cc: virtualization, linux-kernel, mst On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > On 2011年08月27日 10:49, Wanlong Gao wrote: >> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >>> 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. >>> >>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >>> --- >>> include/linux/virtio_ring.h | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >>> index 4a32cb6..fcda152 100644 >>> --- a/include/linux/virtio_ring.h >>> +++ b/include/linux/virtio_ring.h >>> @@ -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 >> > > 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..300af76 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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; } -- 1.7.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:34 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui @ 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin ` (3 more replies) 1 sibling, 4 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 9:52 UTC (permalink / raw) To: wanlong.gao; +Cc: rusty, mst, virtualization, linux-kernel On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > On 2011年08月27日 10:49, Wanlong Gao wrote: >> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >>> 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. >>> >>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >>> --- >>> include/linux/virtio_ring.h | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >>> index 4a32cb6..fcda152 100644 >>> --- a/include/linux/virtio_ring.h >>> +++ b/include/linux/virtio_ring.h >>> @@ -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 >> > > 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..300af76 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:52 ` Wang Sheng-Hui @ 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-28 12:48 ` Wang Sheng-Hui 2011-08-28 12:48 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin ` (2 subsequent siblings) 3 siblings, 2 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2011-08-28 11:31 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: wanlong.gao, rusty, virtualization, linux-kernel On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote: > On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > > On 2011年08月27日 10:49, Wanlong Gao wrote: > >> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: > >>> 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. > >>> > >>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > >>> --- > >>> include/linux/virtio_ring.h | 2 +- > >>> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > >>> index 4a32cb6..fcda152 100644 > >>> --- a/include/linux/virtio_ring.h > >>> +++ b/include/linux/virtio_ring.h > >>> @@ -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 > >> > > > > 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. > > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> Is this a bugfix or just a cleanup? Alignment makes us get the same values with and without, right? > --- > include/linux/virtio_ring.h | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..300af76 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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; > } > -- > 1.7.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-28 11:31 ` Michael S. Tsirkin @ 2011-08-28 12:48 ` Wang Sheng-Hui 2011-08-28 12:48 ` Wang Sheng-Hui 1 sibling, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-28 12:48 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: wanlong.gao, linux-kernel, virtualization On 2011年08月28日 19:31, Michael S. Tsirkin wrote: > On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >>> On 2011年08月27日 10:49, Wanlong Gao wrote: >>>> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >>>>> 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. >>>>> >>>>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >>>>> --- >>>>> include/linux/virtio_ring.h | 2 +- >>>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >>>>> index 4a32cb6..fcda152 100644 >>>>> --- a/include/linux/virtio_ring.h >>>>> +++ b/include/linux/virtio_ring.h >>>>> @@ -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 >>>> >>> >>> 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. >> >> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > > 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. > >> --- >> include/linux/virtio_ring.h | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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; >> } >> -- >> 1.7.1 >> _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-28 12:48 ` Wang Sheng-Hui @ 2011-08-28 12:48 ` Wang Sheng-Hui 1 sibling, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-28 12:48 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: wanlong.gao, rusty, virtualization, linux-kernel On 2011年08月28日 19:31, Michael S. Tsirkin wrote: > On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >>> On 2011年08月27日 10:49, Wanlong Gao wrote: >>>> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >>>>> 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. >>>>> >>>>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >>>>> --- >>>>> include/linux/virtio_ring.h | 2 +- >>>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >>>>> index 4a32cb6..fcda152 100644 >>>>> --- a/include/linux/virtio_ring.h >>>>> +++ b/include/linux/virtio_ring.h >>>>> @@ -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 >>>> >>> >>> 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. >> >> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > > 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. > >> --- >> include/linux/virtio_ring.h | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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; >> } >> -- >> 1.7.1 >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin @ 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-29 2:53 ` Rusty Russell 2011-08-29 2:53 ` Rusty Russell 3 siblings, 0 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2011-08-28 11:31 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: wanlong.gao, linux-kernel, virtualization On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote: > On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > > On 2011年08月27日 10:49, Wanlong Gao wrote: > >> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: > >>> 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. > >>> > >>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> > >>> --- > >>> include/linux/virtio_ring.h | 2 +- > >>> 1 files changed, 1 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > >>> index 4a32cb6..fcda152 100644 > >>> --- a/include/linux/virtio_ring.h > >>> +++ b/include/linux/virtio_ring.h > >>> @@ -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 > >> > > > > 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. > > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> Is this a bugfix or just a cleanup? Alignment makes us get the same values with and without, right? > --- > include/linux/virtio_ring.h | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..300af76 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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; > } > -- > 1.7.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-28 11:31 ` Michael S. Tsirkin @ 2011-08-29 2:53 ` Rusty Russell 2011-08-29 6:51 ` Wang Sheng-Hui ` (3 more replies) 2011-08-29 2:53 ` Rusty Russell 3 siblings, 4 replies; 20+ messages in thread From: Rusty Russell @ 2011-08-29 2:53 UTC (permalink / raw) To: Wang Sheng-Hui, wanlong.gao; +Cc: mst, virtualization, linux-kernel On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: > On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..300af76 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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] > 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 2:53 ` Rusty Russell @ 2011-08-29 6:51 ` Wang Sheng-Hui 2011-08-29 6:51 ` Wang Sheng-Hui ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-29 6:51 UTC (permalink / raw) To: Rusty Russell; +Cc: wanlong.gao, virtualization, linux-kernel, mst On 2011年08月29日 10:53, Rusty Russell wrote: > On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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: Yes. > > * __u16 available[num]; > * __u16 used_event_idx; > * > * // Padding to the next align boundary. > * char pad[]; > * > * [USED] > >> 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 Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 2:53 ` Rusty Russell 2011-08-29 6:51 ` Wang Sheng-Hui @ 2011-08-29 6:51 ` Wang Sheng-Hui 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-29 7:55 ` Wang Sheng-Hui 3 siblings, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-29 6:51 UTC (permalink / raw) To: Rusty Russell; +Cc: wanlong.gao, mst, virtualization, linux-kernel On 2011年08月29日 10:53, Rusty Russell wrote: > On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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: Yes. > > * __u16 available[num]; > * __u16 used_event_idx; > * > * // Padding to the next align boundary. > * char pad[]; > * > * [USED] > >> 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 2:53 ` Rusty Russell 2011-08-29 6:51 ` Wang Sheng-Hui 2011-08-29 6:51 ` Wang Sheng-Hui @ 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-29 7:55 ` Wang Sheng-Hui 3 siblings, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-29 7:55 UTC (permalink / raw) To: Rusty Russell; +Cc: wanlong.gao, virtualization, linux-kernel, mst On 2011年08月29日 10:53, Rusty Russell wrote: > On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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] > >> 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. 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..2a731bd 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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; } -- 1.7.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 2:53 ` Rusty Russell ` (2 preceding siblings ...) 2011-08-29 7:55 ` Wang Sheng-Hui @ 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-30 0:12 ` Rusty Russell 2011-08-30 0:12 ` Rusty Russell 3 siblings, 2 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-29 7:55 UTC (permalink / raw) To: Rusty Russell; +Cc: wanlong.gao, mst, virtualization, linux-kernel On 2011年08月29日 10:53, Rusty Russell wrote: > On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: >> On 2011年08月27日 17:34, Wang Sheng-Hui wrote: >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..300af76 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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] > >> 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. 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..2a731bd 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 7:55 ` Wang Sheng-Hui @ 2011-08-30 0:12 ` Rusty Russell 2011-08-30 0:12 ` Rusty Russell 1 sibling, 0 replies; 20+ messages in thread From: Rusty Russell @ 2011-08-30 0:12 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: wanlong.gao, virtualization, linux-kernel, mst On Mon, 29 Aug 2011 15:55:59 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: > Patch regerated to use sizeof(__u16) instead of 16 in vring_init. > Please check it. Applied. Thanks, Rusty. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-30 0:12 ` Rusty Russell @ 2011-08-30 0:12 ` Rusty Russell 1 sibling, 0 replies; 20+ messages in thread From: Rusty Russell @ 2011-08-30 0:12 UTC (permalink / raw) To: Wang Sheng-Hui; +Cc: wanlong.gao, mst, virtualization, linux-kernel On Mon, 29 Aug 2011 15:55:59 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: > Patch regerated to use sizeof(__u16) instead of 16 in vring_init. > Please check it. Applied. Thanks, Rusty. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 9:52 ` Wang Sheng-Hui ` (2 preceding siblings ...) 2011-08-29 2:53 ` Rusty Russell @ 2011-08-29 2:53 ` Rusty Russell 3 siblings, 0 replies; 20+ messages in thread From: Rusty Russell @ 2011-08-29 2:53 UTC (permalink / raw) To: Wang Sheng-Hui, wanlong.gao; +Cc: virtualization, linux-kernel, mst On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote: > On 2011年08月27日 17:34, Wang Sheng-Hui wrote: > diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h > index 4a32cb6..300af76 100644 > --- a/include/linux/virtio_ring.h > +++ b/include/linux/virtio_ring.h > @@ -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] > 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 Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 9:34 ` Wang Sheng-Hui @ 2011-08-27 9:34 ` Wang Sheng-Hui 1 sibling, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 9:34 UTC (permalink / raw) To: wanlong.gao; +Cc: virtualization, linux-kernel, mst On 2011年08月27日 10:49, Wanlong Gao wrote: > On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote: >> 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. >> >> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> >> --- >> include/linux/virtio_ring.h | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h >> index 4a32cb6..fcda152 100644 >> --- a/include/linux/virtio_ring.h >> +++ b/include/linux/virtio_ring.h >> @@ -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 > 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 Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size @ 2011-08-27 1:07 Wang Sheng-Hui 0 siblings, 0 replies; 20+ messages in thread From: Wang Sheng-Hui @ 2011-08-27 1:07 UTC (permalink / raw) To: rusty, mst, virtualization, linux-kernel 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. Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com> --- include/linux/virtio_ring.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 4a32cb6..fcda152 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -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 */ -- 1.7.1 crossover@crossover-plinux:~/dev/patchwork$ ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-08-30 0:27 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-08-27 1:07 [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size Wang Sheng-Hui 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 2:49 ` Wanlong Gao 2011-08-27 9:34 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-27 9:52 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-28 12:48 ` Wang Sheng-Hui 2011-08-28 12:48 ` Wang Sheng-Hui 2011-08-28 11:31 ` Michael S. Tsirkin 2011-08-29 2:53 ` Rusty Russell 2011-08-29 6:51 ` Wang Sheng-Hui 2011-08-29 6:51 ` Wang Sheng-Hui 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-29 7:55 ` Wang Sheng-Hui 2011-08-30 0:12 ` Rusty Russell 2011-08-30 0:12 ` Rusty Russell 2011-08-29 2:53 ` Rusty Russell 2011-08-27 9:34 ` Wang Sheng-Hui -- strict thread matches above, loose matches on Subject: below -- 2011-08-27 1:07 Wang Sheng-Hui
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.