search for: last_used_idx

...> the implication being that moving event idx back might cause some race > condition? This will cause race condition when tx interrupt is enabled. Consider the following cases 1) tx napi was scheduled 2) start_xmit() call virtqueue_enable_cb_delayed() and disable cb, [used event is vq->last_used_idx + 3/4 pendg bufs] 3) tx napi enable the callback by virtqueue_enable_cb() [ used event is vq->last_used_idx ] After step 3, used event was moved back, unnecessary tx interrupt was triggered. > If yes but please describe the race explicitly. > Is there a bug we need to fix on stable? Loo...

...> the implication being that moving event idx back might cause some race > condition? This will cause race condition when tx interrupt is enabled. Consider the following cases 1) tx napi was scheduled 2) start_xmit() call virtqueue_enable_cb_delayed() and disable cb, [used event is vq->last_used_idx + 3/4 pendg bufs] 3) tx napi enable the callback by virtqueue_enable_cb() [ used event is vq->last_used_idx ] After step 3, used event was moved back, unnecessary tx interrupt was triggered. > If yes but please describe the race explicitly. > Is there a bug we need to fix on stable? Loo...

...nged, 5 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 3b1f89b..1b3929f 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -559,14 +559,17 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) u16 last_used_idx; START_USE(vq); - + last_used_idx = vq->last_used_idx; /* We optimistically turn back on interrupts, then check if there was * more to do. */ /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to * either clear the flags bit or point the event index at the next * entry...

...nged, 5 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 3b1f89b..1b3929f 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -559,14 +559,17 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) u16 last_used_idx; START_USE(vq); - + last_used_idx = vq->last_used_idx; /* We optimistically turn back on interrupts, then check if there was * more to do. */ /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to * either clear the flags bit or point the event index at the next * entry...

...backs. * * Caller must ensure we don't call this with other virtqueue * operations at the same time (except where noted). */ -bool virtqueue_enable_cb(struct virtqueue *_vq) +unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); + u16 last_used_idx; START_USE(vq); @@ -629,15 +631,45 @@ bool virtqueue_enable_cb(struct virtqueue *_vq) * either clear the flags bit or point the event index at the next * entry. Always do both to keep code simple. */ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; - vring_used_event(&...

...backs. * * Caller must ensure we don't call this with other virtqueue * operations at the same time (except where noted). */ -bool virtqueue_enable_cb(struct virtqueue *_vq) +unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); + u16 last_used_idx; START_USE(vq); @@ -629,15 +631,45 @@ bool virtqueue_enable_cb(struct virtqueue *_vq) * either clear the flags bit or point the event index at the next * entry. Always do both to keep code simple. */ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; - vring_used_event(&...

...ing.c b/drivers/virtio/virtio_ring.c index 1ca8890..7729aba 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -89,9 +89,6 @@ struct vring_virtqueue /* Number we've added since last sync. */ unsigned int num_added; - /* Last used index we've seen. */ - u16 last_used_idx; - /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); @@ -285,12 +282,13 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx !=...

...ing.c b/drivers/virtio/virtio_ring.c index 1ca8890..7729aba 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -89,9 +89,6 @@ struct vring_virtqueue /* Number we've added since last sync. */ unsigned int num_added; - /* Last used index we've seen. */ - u16 last_used_idx; - /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); @@ -285,12 +282,13 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx !=...

...cause some race >>> condition? >> This will cause race condition when tx interrupt is enabled. Consider >> the following cases >> >> 1) tx napi was scheduled >> 2) start_xmit() call virtqueue_enable_cb_delayed() and disable cb, [used >> event is vq->last_used_idx + 3/4 pendg bufs] >> 3) tx napi enable the callback by virtqueue_enable_cb() [ used event is >> vq->last_used_idx ] >> >> After step 3, used event was moved back, unnecessary tx interrupt was >> triggered. > Well unnecessary interrupts are safe. But it that is...

...cause some race >>> condition? >> This will cause race condition when tx interrupt is enabled. Consider >> the following cases >> >> 1) tx napi was scheduled >> 2) start_xmit() call virtqueue_enable_cb_delayed() and disable cb, [used >> event is vq->last_used_idx + 3/4 pendg bufs] >> 3) tx napi enable the callback by virtqueue_enable_cb() [ used event is >> vq->last_used_idx ] >> >> After step 3, used event was moved back, unnecessary tx interrupt was >> triggered. > Well unnecessary interrupts are safe. But it that is...

...void *ret; > > START_USE(vq); > @@ -1138,6 +1151,19 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq, > ret = vq->desc_state[id].data; > detach_buf_packed(vq, last_used, id, ctx); > > + wrap_counter = vq->wrap_counter; > + if (vq->last_used_idx > vq->next_avail_idx) > + wrap_counter ^= 1; > + > + /* If we expect an interrupt for the next entry, tell host > + * by writing event index and flush out the write before > + * the read in the next get_buf call. */ > + if (vq->event_flags_shadow == VRING_EVENT_F_DESC)...

...void *ret; > > START_USE(vq); > @@ -1138,6 +1151,19 @@ static void *virtqueue_get_buf_ctx_packed(struct virtqueue *_vq, > ret = vq->desc_state[id].data; > detach_buf_packed(vq, last_used, id, ctx); > > + wrap_counter = vq->wrap_counter; > + if (vq->last_used_idx > vq->next_avail_idx) > + wrap_counter ^= 1; > + > + /* If we expect an interrupt for the next entry, tell host > + * by writing event index and flush out the write before > + * the read in the next get_buf call. */ > + if (vq->event_flags_shadow == VRING_EVENT_F_DESC)...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...

...virtio16(vq->vq.vdev, vq->free_head); vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -448,7 +449,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); } /** @@ -491,8 +492,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) virtio_rmb(vq->weak_barriers); last_used = (vq->last_used_idx &...