search for: virtqueue_enable_cb_prepare

...virtio_ring.c index 5217baf..37d58f8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -607,19 +607,21 @@ void virtqueue_disable_cb(struct virtqueue *_vq) EXPORT_SYMBOL_GPL(virtqueue_disable_cb); /** - * virtqueue_enable_cb - restart callbacks after disable_cb. + * virtqueue_enable_cb_prepare - restart callbacks after disable_cb * @vq: the struct virtqueue we're talking about. * - * This re-enables callbacks; it returns "false" if there are pending - * buffers in the queue, to detect a possible race between the driver - * checking for more work, and enabling callbacks....

...virtio_ring.c index 5217baf..37d58f8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -607,19 +607,21 @@ void virtqueue_disable_cb(struct virtqueue *_vq) EXPORT_SYMBOL_GPL(virtqueue_disable_cb); /** - * virtqueue_enable_cb - restart callbacks after disable_cb. + * virtqueue_enable_cb_prepare - restart callbacks after disable_cb * @vq: the struct virtqueue we're talking about. * - * This re-enables callbacks; it returns "false" if there are pending - * buffers in the queue, to detect a possible race between the driver - * checking for more work, and enabling callbacks....

...gger the races artificially, by adding udelay() in virtqueue_enable_cb() after virtio_mb(). However, we must call napi_complete to clear NAPI_STATE_SCHED before polling the virtqueue for used buffers, otherwise napi_schedule_prep in a callback will fail, causing us to lose RX events. To fix, call virtqueue_enable_cb_prepare with NAPI_STATE_SCHED set (under napi lock), later call virtqueue_poll with NAPI_STATE_SCHED clear (outside the lock). Reported-by: Jason Wang <jasowang at redhat.com> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 inserti...

...gger the races artificially, by adding udelay() in virtqueue_enable_cb() after virtio_mb(). However, we must call napi_complete to clear NAPI_STATE_SCHED before polling the virtqueue for used buffers, otherwise napi_schedule_prep in a callback will fail, causing us to lose RX events. To fix, call virtqueue_enable_cb_prepare with NAPI_STATE_SCHED set (under napi lock), later call virtqueue_poll with NAPI_STATE_SCHED clear (outside the lock). Reported-by: Jason Wang <jasowang at redhat.com> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 inserti...

...changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >> index 1b3929f..d67fbf8 100644 >> --- a/drivers/virtio/virtio_ring.c >> +++ b/drivers/virtio/virtio_ring.c >> @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) >> * entry. Always do both to keep code simple. */ >> vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; >> /* Make sure used event never go backwards */ >> - if (!vring_need_event(vring_used_event(&vq->vring), >> -...

...changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >> index 1b3929f..d67fbf8 100644 >> --- a/drivers/virtio/virtio_ring.c >> +++ b/drivers/virtio/virtio_ring.c >> @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) >> * entry. Always do both to keep code simple. */ >> vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; >> /* Make sure used event never go backwards */ >> - if (!vring_need_event(vring_used_event(&vq->vring), >> -...

...virtio_ring.c index 5217baf..37d58f8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -607,19 +607,21 @@ void virtqueue_disable_cb(struct virtqueue *_vq) EXPORT_SYMBOL_GPL(virtqueue_disable_cb); /** - * virtqueue_enable_cb - restart callbacks after disable_cb. + * virtqueue_enable_cb_prepare - restart callbacks after disable_cb * @vq: the struct virtqueue we're talking about. * - * This re-enables callbacks; it returns "false" if there are pending - * buffers in the queue, to detect a possible race between the driver - * checking for more work, and enabling callbacks....

...virtio_ring.c index 5217baf..37d58f8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -607,19 +607,21 @@ void virtqueue_disable_cb(struct virtqueue *_vq) EXPORT_SYMBOL_GPL(virtqueue_disable_cb); /** - * virtqueue_enable_cb - restart callbacks after disable_cb. + * virtqueue_enable_cb_prepare - restart callbacks after disable_cb * @vq: the struct virtqueue we're talking about. * - * This re-enables callbacks; it returns "false" if there are pending - * buffers in the queue, to detect a possible race between the driver - * checking for more work, and enabling callbacks....

...+-- include/linux/virtio.h | 2 ++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 1b3929f..d67fbf8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) * entry. Always do both to keep code simple. */ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; /* Make sure used event never go backwards */ - if (!vring_need_event(vring_used_event(&vq->vring), - vq->vring.avail->idx, last_used_i...

...+-- include/linux/virtio.h | 2 ++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 1b3929f..d67fbf8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) * entry. Always do both to keep code simple. */ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; /* Make sure used event never go backwards */ - if (!vring_need_event(vring_used_event(&vq->vring), - vq->vring.avail->idx, last_used_i...

On Fri, 2013-07-12 at 23:13 +0200, Wolfram Gloger wrote: > Hi, > > Today I merged M. Tsirkin's patch v2 "virtio_net: fix race in RX VQ > processing": > > http://lkml.indiana.edu/hypermail/linux/kernel/1307.1/00503.html > > into 3.2.48 and lightly tested the result (vhost-net, virtio-disk > and 9pfs using virtio). This sounds like it could be suitable

On Fri, 2013-07-12 at 23:13 +0200, Wolfram Gloger wrote: > Hi, > > Today I merged M. Tsirkin's patch v2 "virtio_net: fix race in RX VQ > processing": > > http://lkml.indiana.edu/hypermail/linux/kernel/1307.1/00503.html > > into 3.2.48 and lightly tested the result (vhost-net, virtio-disk > and 9pfs using virtio). This sounds like it could be suitable

...t;>> >>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>>> index 1b3929f..d67fbf8 100644 >>>> --- a/drivers/virtio/virtio_ring.c >>>> +++ b/drivers/virtio/virtio_ring.c >>>> @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) >>>> * entry. Always do both to keep code simple. */ >>>> vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; >>>> /* Make sure used event never go backwards */ >>>> - if (!vring_need_event(vring_used_event(&...

...t;>> >>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>>> index 1b3929f..d67fbf8 100644 >>>> --- a/drivers/virtio/virtio_ring.c >>>> +++ b/drivers/virtio/virtio_ring.c >>>> @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) >>>> * entry. Always do both to keep code simple. */ >>>> vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; >>>> /* Make sure used event never go backwards */ >>>> - if (!vring_need_event(vring_used_event(&...

...2 ++ > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 1b3929f..d67fbf8 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) > * entry. Always do both to keep code simple. */ > vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; > /* Make sure used event never go backwards */ > - if (!vring_need_event(vring_used_event(&vq->vring), > - vq->vring.a...

...poll() always enabled interrupts before complete, and when napi was rescheduled within napi_complete_done() it did not disable interrupts. This caused more interrupts when event idx is disabled. According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ processing") we cannot place virtqueue_enable_cb_prepare() after NAPI_STATE_SCHED is cleared, so disable interrupts again if napi_complete_done() returned false. Tested with vhost-user of OVS 2.7 on host, which does not have the event idx feature. * Before patch: $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472 MIGRATED UDP STREAM TEST from...

...poll() always enabled interrupts before complete, and when napi was rescheduled within napi_complete_done() it did not disable interrupts. This caused more interrupts when event idx is disabled. According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ processing") we cannot place virtqueue_enable_cb_prepare() after NAPI_STATE_SCHED is cleared, so disable interrupts again if napi_complete_done() returned false. Tested with vhost-user of OVS 2.7 on host, which does not have the event idx feature. * Before patch: $ netperf -t UDP_STREAM -H 192.168.150.253 -l 60 -- -m 1472 MIGRATED UDP STREAM TEST from...

...eletions(-) > >> > >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > >> index 1b3929f..d67fbf8 100644 > >> --- a/drivers/virtio/virtio_ring.c > >> +++ b/drivers/virtio/virtio_ring.c > >> @@ -567,14 +567,32 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq) > >> * entry. Always do both to keep code simple. */ > >> vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; > >> /* Make sure used event never go backwards */ > >> - if (!vring_need_event(vring_used_event(&vq->v...

...ks - * @vq: the struct virtqueue we're talking about. + * @_vq: the struct virtqueue we're talking about. * * Note that this is not necessarily synchronous, hence unreliable and only * useful as an optimization. @@ -1936,7 +1937,7 @@ EXPORT_SYMBOL_GPL(virtqueue_disable_cb); /** * virtqueue_enable_cb_prepare - restart callbacks after disable_cb - * @vq: the struct virtqueue we're talking about. + * @_vq: the struct virtqueue we're talking about. * * This re-enables callbacks; it returns current queue state * in an opaque unsigned value. This value should be later tested by @@ -1957,7 +195...

...when >> napi was rescheduled within napi_complete_done() it did not disable >> interrupts. >> This caused more interrupts when event idx is disabled. >> >> According to commit cbdadbbf0c79 ("virtio_net: fix race in RX VQ >> processing") we cannot place virtqueue_enable_cb_prepare() after >> NAPI_STATE_SCHED is cleared, so disable interrupts again if >> napi_complete_done() returned false. >> >> Tested with vhost-user of OVS 2.7 on host, which does not have the event >> idx feature. >> >> * Before patch: >> >> $ netperf -...