search for: event_idx

...ing size, but old looks not. > > > > > > What vring_need_event() cares is the distance between > > > `new` and `old`, i.e. vq->num_added. So I think there > > > is nothing wrong with `old`. But the calculation of the > > > distance between `new` and `event_idx` isn't right when > > > `new` wraps. How do you think about the below code: > > > > > > wrap_counter = off_wrap >> 15; > > > event_idx = off_wrap & ~(1<<15); > > > if (wrap_counter != vq->wrap_counter) > > > event_id...

...ing size, but old looks not. > > > > > > What vring_need_event() cares is the distance between > > > `new` and `old`, i.e. vq->num_added. So I think there > > > is nothing wrong with `old`. But the calculation of the > > > distance between `new` and `event_idx` isn't right when > > > `new` wraps. How do you think about the below code: > > > > > > wrap_counter = off_wrap >> 15; > > > event_idx = off_wrap & ~(1<<15); > > > if (wrap_counter != vq->wrap_counter) > > > event_id...

...gt; > > > > > What vring_need_event() cares is the distance between > > > > > `new` and `old`, i.e. vq->num_added. So I think there > > > > > is nothing wrong with `old`. But the calculation of the > > > > > distance between `new` and `event_idx` isn't right when > > > > > `new` wraps. How do you think about the below code: > > > > > > > > > > wrap_counter = off_wrap >> 15; > > > > > event_idx = off_wrap & ~(1<<15); > > > > > if (wrap_counter...

...gt; > > > > > What vring_need_event() cares is the distance between > > > > > `new` and `old`, i.e. vq->num_added. So I think there > > > > > is nothing wrong with `old`. But the calculation of the > > > > > distance between `new` and `event_idx` isn't right when > > > > > `new` wraps. How do you think about the below code: > > > > > > > > > > wrap_counter = off_wrap >> 15; > > > > > event_idx = off_wrap & ~(1<<15); > > > > > if (wrap_counter...

...nd event are in the > > unit of descriptor ring size, but old looks not. > > What vring_need_event() cares is the distance between > `new` and `old`, i.e. vq->num_added. So I think there > is nothing wrong with `old`. But the calculation of the > distance between `new` and `event_idx` isn't right when > `new` wraps. How do you think about the below code: > > wrap_counter = off_wrap >> 15; > event_idx = off_wrap & ~(1<<15); > if (wrap_counter != vq->wrap_counter) > event_idx -= vq->vring_packed.num; > > needs_kick = vrin...

...nd event are in the > > unit of descriptor ring size, but old looks not. > > What vring_need_event() cares is the distance between > `new` and `old`, i.e. vq->num_added. So I think there > is nothing wrong with `old`. But the calculation of the > distance between `new` and `event_idx` isn't right when > `new` wraps. How do you think about the below code: > > wrap_counter = off_wrap >> 15; > event_idx = off_wrap & ~(1<<15); > if (wrap_counter != vq->wrap_counter) > event_idx -= vq->vring_packed.num; > > needs_kick = vrin...

...pi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h index a3318f3..915980a 100644 --- a/include/uapi/linux/virtio_ring.h +++ b/include/uapi/linux/virtio_ring.h @@ -155,7 +155,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align) } /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */ -/* Assuming a given event_idx value from the other size, if +/* Assuming a given event_idx value from the other side, if * we have just incremented index from old to new_idx, * should we trigger an event? */ static inline int vring_need_event(__u16 event_idx, __u16 new_...

...pi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h index a3318f3..915980a 100644 --- a/include/uapi/linux/virtio_ring.h +++ b/include/uapi/linux/virtio_ring.h @@ -155,7 +155,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align) } /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */ -/* Assuming a given event_idx value from the other size, if +/* Assuming a given event_idx value from the other side, if * we have just incremented index from old to new_idx, * should we trigger an event? */ static inline int vring_need_event(__u16 event_idx, __u16 new_...

...ot. > > > > > > > > What vring_need_event() cares is the distance between > > > > `new` and `old`, i.e. vq->num_added. So I think there > > > > is nothing wrong with `old`. But the calculation of the > > > > distance between `new` and `event_idx` isn't right when > > > > `new` wraps. How do you think about the below code: > > > > > > > > wrap_counter = off_wrap >> 15; > > > > event_idx = off_wrap & ~(1<<15); > > > > if (wrap_counter != vq->wrap_counter)...

...> > > What vring_need_event() cares is the distance between > > > > > > `new` and `old`, i.e. vq->num_added. So I think there > > > > > > is nothing wrong with `old`. But the calculation of the > > > > > > distance between `new` and `event_idx` isn't right when > > > > > > `new` wraps. How do you think about the below code: > > > > > > > > > > > > wrap_counter = off_wrap >> 15; > > > > > > event_idx = off_wrap & ~(1<<15); > > > > &g...

...> unit of descriptor ring size, but old looks not. > > > > What vring_need_event() cares is the distance between > > `new` and `old`, i.e. vq->num_added. So I think there > > is nothing wrong with `old`. But the calculation of the > > distance between `new` and `event_idx` isn't right when > > `new` wraps. How do you think about the below code: > > > > wrap_counter = off_wrap >> 15; > > event_idx = off_wrap & ~(1<<15); > > if (wrap_counter != vq->wrap_counter) > > event_idx -= vq->vring_packed.num;...

On Wed, Jul 11, 2018 at 10:27:10AM +0800, Tiwei Bie wrote: > This commit introduces the EVENT_IDX support in packed ring. > > Signed-off-by: Tiwei Bie <tiwei.bie at intel.com> Besides the usual comment about hard-coded constants like <<15: does this actually do any good for performance? We don't have to if we do not want to. > --- > drivers/virtio/virtio_ring.c |...

...tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> cannot. Then busyloop will run without suppressing notification. >> Another example is the case where handle_tx() tries to enable >> notification but avail idx is advanced so disables it again. This case >> also lead to the same situation with event_idx. >...

...tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> cannot. Then busyloop will run without suppressing notification. >> Another example is the case where handle_tx() tries to enable >> notification but avail idx is advanced so disables it again. This case >> also lead to the same situation with event_idx. >...

This commit introduces the EVENT_IDX support in packed ring. Signed-off-by: Tiwei Bie <tiwei.bie at intel.com> --- drivers/virtio/virtio_ring.c | 73 ++++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index cb833a82054b...

This commit introduces the EVENT_IDX support in packed ring. Signed-off-by: Tiwei Bie <tiwei.bie at intel.com> --- drivers/virtio/virtio_ring.c | 73 ++++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index f317b485ba54...

This commit introduces the EVENT_IDX support in packed ring. Signed-off-by: Tiwei Bie <tiwei.bie at intel.com> --- drivers/virtio/virtio_ring.c | 74 ++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 983ce5ffda1b...

On 2018?05?03? 10:09, Tiwei Bie wrote: >>>> So how about we use the straightforward way then? >>> You mean we do new += vq->vring_packed.num instead >>> of event_idx -= vq->vring_packed.num before calling >>> vring_need_event()? >>> >>> The problem is that, the second param (new_idx) of >>> vring_need_event() will be used for: >>> >>> (__u16)(new_idx - event_idx - 1) >>> (__u16)(new_idx - old) &g...

On 2018?05?03? 10:09, Tiwei Bie wrote: >>>> So how about we use the straightforward way then? >>> You mean we do new += vq->vring_packed.num instead >>> of event_idx -= vq->vring_packed.num before calling >>> vring_need_event()? >>> >>> The problem is that, the second param (new_idx) of >>> vring_need_event() will be used for: >>> >>> (__u16)(new_idx - event_idx - 1) >>> (__u16)(new_idx - old) &g...

...should use READ_ONCE() to prevent compiler from re-reading. > + off_wrap = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot & 0xffff)); > flags = virtio16_to_cpu(_vq->vdev, (__virtio16)(snapshot >> 16)) & 0x3; > > + wrap_counter = off_wrap >> 15; > + event_idx = off_wrap & ~(1 << 15); > + if (wrap_counter != vq->avail_wrap_counter) > + event_idx -= vq->vring_packed.num; Thanks