search for: virtqueue_enable_cb_delayed_packed

...ment. Tx queue will be stopped if there's not > enough freed buffers after recheck. > > Signed-off-by: Marvin Liu <yong.liu at intel.com> So as I wrote separately, I don't think the patch is correct. And it doesn't look like it was tested properly. However, I do think virtqueue_enable_cb_delayed_packed is implemented incorrectly in case of chained s/g and ring that is close to being full. It's just that chained only happens on OOM at the moment, so it's hard to trigger. virtio_ring.c can be hacked to force chained to make it trigger more. And with napi_tx the default this function is bar...

...; > > Both of ping and netperf worked as expected. > > > > v1 -> v2: > > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > - Add comments related to ccw (Jason); > > > > RFC (v6) -> v1: > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > when event idx is off (Jason); > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > - Test the state of the desc at used_idx instead of last_used_idx > > in virtqueue_enable_cb_delayed_packed() (Jason); > > - Save wrap counter (as p...

From: Marvin Liu <yong.liu at intel.com> When VIRTIO_F_RING_EVENT_IDX is negotiated, virtio devices can use virtqueue_enable_cb_delayed_packed to reduce the number of device interrupts. At the moment, this is the case for virtio-net when the napi_tx module parameter is set to false. In this case, the virtio driver selects an event offset and expects that the device will send a notification when rolling over the event offset in the ring....

...t; > > > > > v1 -> v2: > > > > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > > > - Add comments related to ccw (Jason); > > > > > > > > RFC (v6) -> v1: > > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > > > when event idx is off (Jason); > > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > > > - Test the state of the desc at used_idx instead of last_used_idx > > > > in virtqueue_enable_cb_delayed_packed() (Jas...

...values (MST); > - Drop the notify workaround for QEMU's tx-timer in packed ring (MST); > > v1 -> v2: > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > - Add comments related to ccw (Jason); > > RFC v6 -> v1: > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > when event idx is off (Jason); > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > - Test the state of the desc at used_idx instead of last_used_idx > in virtqueue_enable_cb_delayed_packed() (Jason); > - Save wrap counter (as part of queue state) in th...

...; > > > > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > > > > > - Add comments related to ccw (Jason); > > > > > > > > > > > > RFC (v6) -> v1: > > > > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > > > > > when event idx is off (Jason); > > > > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > > > > > - Test the state of the desc at used_idx instead of last_used_idx > > > > > > in...

...values (MST); > - Drop the notify workaround for QEMU's tx-timer in packed ring (MST); > > v1 -> v2: > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > - Add comments related to ccw (Jason); > > RFC v6 -> v1: > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > when event idx is off (Jason); > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > - Test the state of the desc at used_idx instead of last_used_idx > in virtqueue_enable_cb_delayed_packed() (Jason); > - Save wrap counter (as part of queue state) in th...

...ed-off-by: Marvin Liu <yong.liu at intel.com> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index bdc08244a648..a8041e451e9e 100644 > --- a/drivers/virtio/virtio_ring.c > +++ 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; > } Is all this theorectical? Or did you actuall...

...rf worked as expected. > > > > > > v1 -> v2: > > > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > > - Add comments related to ccw (Jason); > > > > > > RFC (v6) -> v1: > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > > when event idx is off (Jason); > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > > - Test the state of the desc at used_idx instead of last_used_idx > > > in virtqueue_enable_cb_delayed_packed() (Jason); > > >...

...ing last > used index as replacement. Tx queue will be stopped if there's not > enough freed buffers after recheck. > > Signed-off-by: Marvin Liu <yong.liu at intel.com> OK I rewrote the commit log slightly: When VIRTIO_F_RING_EVENT_IDX is negotiated, virtio devices can use virtqueue_enable_cb_delayed_packed to reduce the number of device interrupts. At the moment, this is the case for virtio-net when the napi_tx module parameter is set to false. In this case, the virtio driver selects an event offset in the ring and expects that the device will send a notification when rolling over the event offset...

...gt; >>>>> v1 -> v2: >>>>> - Use READ_ONCE() to read event off_wrap and flags together (Jason); >>>>> - Add comments related to ccw (Jason); >>>>> >>>>> RFC (v6) -> v1: >>>>> - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() >>>>> when event idx is off (Jason); >>>>> - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); >>>>> - Test the state of the desc at used_idx instead of last_used_idx >>>>> in virtqueue_enable_cb_delayed_packed(...

...fy workaround for QEMU's tx-timer in packed ring (MST); > > > > v1 -> v2: > > - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > - Add comments related to ccw (Jason); > > > > RFC v6 -> v1: > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > when event idx is off (Jason); > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > - Test the state of the desc at used_idx instead of last_used_idx > > in virtqueue_enable_cb_delayed_packed() (Jason); > > - Save wrap counter (as p...

...; - Use READ_ONCE() to read event off_wrap and flags together (Jason); > > > > > > > - Add comments related to ccw (Jason); > > > > > > > > > > > > > > RFC (v6) -> v1: > > > > > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > > > > > > when event idx is off (Jason); > > > > > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > > > > > > - Test the state of the desc at used_idx instead of last_used_idx > > > >...

.../virtio_ring.c >> b/drivers/virtio/virtio_ring.c >>>>> index bdc08244a648..a8041e451e9e 100644 >>>>> --- a/drivers/virtio/virtio_ring.c >>>>> +++ 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; >>>>> -...

...ned-off-by: Marvin Liu <yong.liu at intel.com> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index bdc08244a648..a8041e451e9e 100644 > --- a/drivers/virtio/virtio_ring.c > +++ 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_fl...

...16 d1, d2; > > ??? if (wrap != vq->used_wrap_counter) > ??? ??? d1 = new + vq->num - off - 1; Just to draw your attention (maybe you have already noticed this). In this case (i.e. wrap != vq->used_wrap_counter), it's also possible that (off < new) is true. Because, when virtqueue_enable_cb_delayed_packed() is used, `off` is calculated in driver in a way like this: off = vq->last_used_idx + bufs; if (off >= vq->vring_packed.num) { off -= vq->vring_packed.num; wrap_counter ^= 1; } And when `new` (in vhost) is close to vq->num. The vq->last_used_idx + bufs (in driver) can be...

...16 d1, d2; > > ??? if (wrap != vq->used_wrap_counter) > ??? ??? d1 = new + vq->num - off - 1; Just to draw your attention (maybe you have already noticed this). In this case (i.e. wrap != vq->used_wrap_counter), it's also possible that (off < new) is true. Because, when virtqueue_enable_cb_delayed_packed() is used, `off` is calculated in driver in a way like this: off = vq->last_used_idx + bufs; if (off >= vq->vring_packed.num) { off -= vq->vring_packed.num; wrap_counter ^= 1; } And when `new` (in vhost) is close to vq->num. The vq->last_used_idx + bufs (in driver) can be...

...>> >>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>> index bdc08244a648..a8041e451e9e 100644 >>> --- a/drivers/virtio/virtio_ring.c >>> +++ 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; &...

...to read event off_wrap and flags together (Jason); > > > > > > > > - Add comments related to ccw (Jason); > > > > > > > > > > > > > > > > RFC (v6) -> v1: > > > > > > > > - Avoid extra virtio_wmb() in virtqueue_enable_cb_delayed_packed() > > > > > > > > when event idx is off (Jason); > > > > > > > > - Fix bufs calculation in virtqueue_enable_cb_delayed_packed() (Jason); > > > > > > > > - Test the state of the desc at used_idx instead of last_used_idx >...

...1; > > Just to draw your attention (maybe you have already > > noticed this). > > I miss this, thanks! > > > > > In this case (i.e. wrap != vq->used_wrap_counter), > > it's also possible that (off < new) is true. Because, > > > > when virtqueue_enable_cb_delayed_packed() is used, > > `off` is calculated in driver in a way like this: > > > > off = vq->last_used_idx + bufs; > > if (off >= vq->vring_packed.num) { > > off -= vq->vring_packed.num; > > wrap_counter ^= 1; > > } > > > > And when `...