search for: used_wrap_counter

...distance correctly between event and new, we just > > need to compare the warp counter and return false if it doesn't match > > without the need to try to add vq.num here. > > > > Thanks > > Sorry, looks like the following should work, we need add vq.num if > used_wrap_counter does not match: > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, > ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, > ??? ??? ??? ??? ??? ? __u16 old) > { > ??? bool wrap = off_wrap >> 15; > ??? int off = off_wrap & ~(1 << 15); > ???...

...distance correctly between event and new, we just > > need to compare the warp counter and return false if it doesn't match > > without the need to try to add vq.num here. > > > > Thanks > > Sorry, looks like the following should work, we need add vq.num if > used_wrap_counter does not match: > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, > ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, > ??? ??? ??? ??? ??? ? __u16 old) > { > ??? bool wrap = off_wrap >> 15; > ??? int off = off_wrap & ~(1 << 15); > ???...

...; > > > need to compare the warp counter and return false if it doesn't match > > > > without the need to try to add vq.num here. > > > > > > > > Thanks > > > Sorry, looks like the following should work, we need add vq.num if > > > used_wrap_counter does not match: > > > > > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, > > > ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, > > > ??? ??? ??? ??? ??? ? __u16 old) > > > { > > > ??? bool wrap = off_wrap >> 1...

...; > > > need to compare the warp counter and return false if it doesn't match > > > > without the need to try to add vq.num here. > > > > > > > > Thanks > > > Sorry, looks like the following should work, we need add vq.num if > > > used_wrap_counter does not match: > > > > > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, > > > ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, > > > ??? ??? ??? ??? ??? ? __u16 old) > > > { > > > ??? bool wrap = off_wrap >> 1...

...and return false if it doesn't match > > > > > > without the need to try to add vq.num here. > > > > > > > > > > > > Thanks > > > > > Sorry, looks like the following should work, we need add vq.num if > > > > > used_wrap_counter does not match: > > > > > > > > > > static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, > > > > > ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, > > > > > ??? ??? ??? ??? ??? ? __u16 old) > > > > > {...

...re > + * the read in the next get_buf call. */ > + if (vq->event_flags_shadow == VRING_EVENT_F_DESC) > + virtio_store_mb(vq->weak_barriers, > + &vq->vring_packed.driver->off_wrap, > + cpu_to_virtio16(_vq->vdev, vq->last_used_idx | > + ((u16)vq->used_wrap_counter << 15))); > + > #ifdef DEBUG > vq->last_add_time_valid = false; > #endif > @@ -1213,8 +1235,18 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq) > /* We optimistically turn back on interrupts, then check if there was > * more to do....

...rectly between event and new, we just >>> need to compare the warp counter and return false if it doesn't match >>> without the need to try to add vq.num here. >>> >>> Thanks >> Sorry, looks like the following should work, we need add vq.num if >> used_wrap_counter does not match: >> >> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, >> ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, >> ??? ??? ??? ??? ??? ? __u16 old) >> { >> ??? bool wrap = off_wrap >> 15; >> ??? int off = off_wrap &...

...t;>> need to compare the warp counter and return false if it doesn't match >>>>> without the need to try to add vq.num here. >>>>> >>>>> Thanks >>>> Sorry, looks like the following should work, we need add vq.num if >>>> used_wrap_counter does not match: >>>> >>>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq, >>>> ??? ??? ??? ??? ??? ? __u16 off_wrap, __u16 new, >>>> ??? ??? ??? ??? ??? ? __u16 old) >>>> { >>>> ??? bool wrap = off_w...

On 2018?05?03? 21:54, Tiwei Bie wrote: > On Thu, May 03, 2018 at 03:25:29PM +0800, Jason Wang wrote: >> 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 >>>>>

On 2018?05?03? 21:54, Tiwei Bie wrote: > On Thu, May 03, 2018 at 03:25:29PM +0800, Jason Wang wrote: >> 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 >>>>>

...x and flush out the write before + * the read in the next get_buf call. */ + if (vq->event_flags_shadow == VRING_EVENT_F_DESC) + virtio_store_mb(vq->weak_barriers, + &vq->vring_packed.driver->off_wrap, + cpu_to_virtio16(_vq->vdev, vq->last_used_idx | + ((u16)vq->used_wrap_counter << 15))); + #ifdef DEBUG vq->last_add_time_valid = false; #endif @@ -1213,8 +1235,18 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq) /* We optimistically turn back on interrupts, then check if there was * more to do. */ + if (vq->event) { + vq-&g...

...x and flush out the write before + * the read in the next get_buf call. */ + if (vq->event_flags_shadow == VRING_EVENT_F_DESC) + virtio_store_mb(vq->weak_barriers, + &vq->vring_packed.driver->off_wrap, + cpu_to_virtio16(_vq->vdev, vq->last_used_idx | + ((u16)vq->used_wrap_counter << 15))); + #ifdef DEBUG vq->last_add_time_valid = false; #endif @@ -1213,8 +1235,18 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq) /* We optimistically turn back on interrupts, then check if there was * more to do. */ + if (vq->event) { + vq-&g...

...> @@ -1499,9 +1499,6 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) > * counter first before updating event flags. > */ > virtio_wmb(vq->weak_barriers); > - } else { > - used_idx = vq->last_used_idx; > - wrap_counter = vq->packed.used_wrap_counter; > } Is all this theorectical? Or did you actually see a problem and then fixed it? Because as far as I could see after this patch and with event index off, used_idx and wrap_counter will be used without being initialized. OTOH the behaviour with event index on is completely unaffected. &...

...ed; + struct vring_used_elem used; mutex_lock(&vq->mutex); sock = vq->private_data; @@ -494,6 +496,8 @@ static void handle_tx(struct vhost_net *net) vhost_zerocopy_signal_used(net, vq); + used.idx = vq->last_avail_idx & (vq->num - 1); + used.wrap_counter = vq->used_wrap_counter; head = vhost_net_tx_get_vq_desc(net, vq, vq->iov, ARRAY_SIZE(vq->iov), &out, &in); @@ -515,6 +519,8 @@ static void handle_tx(struct vhost_net *net) } /* Skip header. TODO: support TSO. */ len = iov_length(vq->iov, out); + used.id = head; + used.len = 0;...

...> @@ -1499,9 +1499,6 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) > * counter first before updating event flags. > */ > virtio_wmb(vq->weak_barriers); > - } else { > - used_idx = vq->last_used_idx; > - wrap_counter = vq->packed.used_wrap_counter; > } > > if (vq->packed.event_flags_shadow == VRING_PACKED_EVENT_FLAG_DISABLE) { > @@ -1518,7 +1515,9 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) > */ > virtio_mb(vq->weak_barriers); > > - if (is_used_desc_packed(vq, used_idx,...

...+++ b/drivers/virtio/virtio_ring.c @@ -1499,9 +1499,6 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) * counter first before updating event flags. */ virtio_wmb(vq->weak_barriers); - } else { - used_idx = vq->last_used_idx; - wrap_counter = vq->packed.used_wrap_counter; } if (vq->packed.event_flags_shadow == VRING_PACKED_EVENT_FLAG_DISABLE) { @@ -1518,7 +1515,9 @@ static bool virtqueue_enable_cb_delayed_packed(struct virtqueue *_vq) */ virtio_mb(vq->weak_barriers); - if (is_used_desc_packed(vq, used_idx, wrap_counter)) { + if (is_used_desc_pack...

...x and flush out the write before + * the read in the next get_buf call. */ + if (vq->event_flags_shadow == VRING_EVENT_F_DESC) + virtio_store_mb(vq->weak_barriers, + &vq->vring_packed.driver->off_wrap, + cpu_to_virtio16(_vq->vdev, vq->last_used_idx | + ((u16)vq->used_wrap_counter << 15))); + #ifdef DEBUG vq->last_add_time_valid = false; #endif @@ -1204,9 +1226,20 @@ static unsigned virtqueue_enable_cb_prepare_packed(struct virtqueue *_vq) /* We optimistically turn back on interrupts, then check if there was * more to do. */ + /* Depending on the VIRTIO_R...

...+ vring_unmap_desc_packed(vq, &desc[i]); + } + kfree(desc); + vq->desc_state_packed[id].indir_desc = NULL; + } else if (ctx) { + *ctx = vq->desc_state_packed[id].indir_desc; + } +} + +static inline bool is_used_desc_packed(const struct vring_virtqueue *vq, + u16 idx, bool used_wrap_counter) +{ + u16 flags; + bool avail, used; + + flags = virtio16_to_cpu(vq->vq.vdev, + vq->vring_packed.desc[idx].flags); + avail = !!(flags & VRING_DESC_F_AVAIL); + used = !!(flags & VRING_DESC_F_USED); + + return avail == used && used == used_wrap_counter; } static inline bo...

...;>>> * counter first before updating event flags. >>>>> */ >>>>> virtio_wmb(vq->weak_barriers); >>>>> - } else { >>>>> - used_idx = vq->last_used_idx; >>>>> - wrap_counter = vq->packed.used_wrap_counter; >>>>> } >>>>> >>>>> if (vq->packed.event_flags_shadow == >> VRING_PACKED_EVENT_FLAG_DISABLE) >>>> { >>>>> @@ -1518,7 +1515,9 @@ static bool >>>> virtqueue_enable_cb_delayed_packed(struct virtqueu...

...of > VIRTQ_DESC_F_USED/VIRTQ_- > DESC_F_AVAIL. Other techniques to detect > VIRTQ_DESC_F_AVAIL/VIRTQ_DESC_F_USED bit changes > might also be possible. > " > > So it looks to me it was not sufficient, looking at the example codes in > spec, do we need to track last seen used_wrap_counter here? I don't think we have to track used_wrap_counter in driver. There was a discussion on this: https://lists.oasis-open.org/archives/virtio-dev/201802/msg00177.html And after that, below sentence was added (it's also in the above words you quoted): """ Other techniques...