search for: virtqueue_disable_cb

On Mon, Feb 09, 2015 at 03:39:21AM -0500, Jason Wang wrote: > Currently, we do nothing to prevent the callbacks in > virtqueue_disable_cb() when event index is used. This may cause > spurious interrupts which may damage the performance. This patch tries > to publish avail event as the used even to prevent the callbacks. > > Signed-off-by: Jason Wang <jasowang at redhat.com> I'm surprised that this ever happens...

On Mon, Feb 09, 2015 at 03:39:21AM -0500, Jason Wang wrote: > Currently, we do nothing to prevent the callbacks in > virtqueue_disable_cb() when event index is used. This may cause > spurious interrupts which may damage the performance. This patch tries > to publish avail event as the used even to prevent the callbacks. > > Signed-off-by: Jason Wang <jasowang at redhat.com> I'm surprised that this ever happens...

Currently, we do nothing to prevent the callbacks in virtqueue_disable_cb() when event index is used. This may cause spurious interrupts which may damage the performance. This patch tries to publish avail event as the used even to prevent the callbacks. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/virtio/virtio_ring.c | 2 ++ 1 file changed, 2 i...

Currently, we do nothing to prevent the callbacks in virtqueue_disable_cb() when event index is used. This may cause spurious interrupts which may damage the performance. This patch tries to publish avail event as the used even to prevent the callbacks. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/virtio/virtio_ring.c | 2 ++ 1 file changed, 2 i...

...update_end(&stats->tx_syncp); + + dev_kfree_skb_any(skb); + sent++; + } + + return sent; +} + static void skb_xmit_done(struct virtqueue *vq) { struct virtnet_info *vi = vq->vdev->priv; + struct send_queue *sq = &vi->sq[vq2txq(vq)]; - /* Suppress further interrupts. */ - virtqueue_disable_cb(vq); - - /* We were probably waiting for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) { + __napi_schedule(&sq->napi); + } } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -766,6 +791,37 @@...

...update_end(&stats->tx_syncp); + + dev_kfree_skb_any(skb); + sent++; + } + + return sent; +} + static void skb_xmit_done(struct virtqueue *vq) { struct virtnet_info *vi = vq->vdev->priv; + struct send_queue *sq = &vi->sq[vq2txq(vq)]; - /* Suppress further interrupts. */ - virtqueue_disable_cb(vq); - - /* We were probably waiting for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) { + __napi_schedule(&sq->napi); + } } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -766,6 +791,37 @@...

...deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 019da01..971931e5 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1182,11 +1182,6 @@ static int virtnet_freeze(struct virtio_device *vdev) { struct virtnet_info *vi = vdev->priv; - virtqueue_disable_cb(vi->rvq); - virtqueue_disable_cb(vi->svq); - if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) - virtqueue_disable_cb(vi->cvq); - netif_device_detach(vi->dev); cancel_delayed_work_sync(&vi->refill); -- 1.7.9.111.gf3fb0

...deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 019da01..971931e5 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1182,11 +1182,6 @@ static int virtnet_freeze(struct virtio_device *vdev) { struct virtnet_info *vi = vdev->priv; - virtqueue_disable_cb(vi->rvq); - virtqueue_disable_cb(vi->svq); - if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) - virtqueue_disable_cb(vi->cvq); - netif_device_detach(vi->dev); cancel_delayed_work_sync(&vi->refill); -- 1.7.9.111.gf3fb0

...ote: >> > static void skb_xmit_done(struct virtqueue *vq) >> > { >> > struct virtnet_info *vi = vq->vdev->priv; >> > + struct send_queue *sq = &vi->sq[vq2txq(vq)]; >> > >> > - /* Suppress further interrupts. */ >> > - virtqueue_disable_cb(vq); >> > - > One note here: current code seems racy because of doing > virtqueue_disable_cb from skb_xmit_done that I'm dropping here: there's > no guarantee we don't get an interrupt while tx ring is running, and if > that happens we can end up with interrupts dis...

...ote: >> > static void skb_xmit_done(struct virtqueue *vq) >> > { >> > struct virtnet_info *vi = vq->vdev->priv; >> > + struct send_queue *sq = &vi->sq[vq2txq(vq)]; >> > >> > - /* Suppress further interrupts. */ >> > - virtqueue_disable_cb(vq); >> > - > One note here: current code seems racy because of doing > virtqueue_disable_cb from skb_xmit_done that I'm dropping here: there's > no guarantee we don't get an interrupt while tx ring is running, and if > that happens we can end up with interrupts dis...

...alse); > > > > > > > > if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) { > > > > > > > > netif_start_subqueue(dev, qnum); > > > > > > > > virtqueue_disable_cb(sq->vq); > > > > > > > > } > > > > > > > > > > > > > > The code example here is out of date, make sure your tree has this: > > > > > > > > > > > > also, I will correct it...

Hello, The first patch here fixes the race seen by Miche. He hasn't yet reported back if this fixes the races he saw, but Joy Pu from Red Hat tested this patch with hot-plugging/unplugging ports in a loop. Before this patch, he saw some freezes as well as sysfs warnings. After applying the patch, all was well. The second patch can be folded into the series fixing S4 for virtio-console. It

Hello, The first patch here fixes the race seen by Miche. He hasn't yet reported back if this fixes the races he saw, but Joy Pu from Red Hat tested this patch with hot-plugging/unplugging ports in a loop. Before this patch, he saw some freezes as well as sysfs warnings. After applying the patch, all was well. The second patch can be folded into the series fixing S4 for virtio-console. It

..._end(&stats->tx_syncp); + + dev_kfree_skb_any(skb); + packets++; + } + + return packets; +} + static void skb_xmit_done(struct virtqueue *vq) { struct virtnet_info *vi = vq->vdev->priv; + struct send_queue *sq = &vi->sq[vq2txq(vq)]; - /* Suppress further interrupts. */ - virtqueue_disable_cb(vq); - - /* We were probably waiting for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) + __napi_schedule(&sq->napi); } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -774,6 +798,37 @@ again...

..._end(&stats->tx_syncp); + + dev_kfree_skb_any(skb); + packets++; + } + + return packets; +} + static void skb_xmit_done(struct virtqueue *vq) { struct virtnet_info *vi = vq->vdev->priv; + struct send_queue *sq = &vi->sq[vq2txq(vq)]; - /* Suppress further interrupts. */ - virtqueue_disable_cb(vq); - - /* We were probably waiting for more output buffers. */ - netif_wake_subqueue(vi->dev, vq2txq(vq)); + if (napi_schedule_prep(&sq->napi)) + __napi_schedule(&sq->napi); } static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx) @@ -774,6 +798,37 @@ again...

...f_tx_start_queue(txq); > + > + return packets; > +} > + > static void skb_xmit_done(struct virtqueue *vq) > { > struct virtnet_info *vi = vq->vdev->priv; > + struct send_queue *sq = &vi->sq[vq2txq(vq)]; > > - /* Suppress further interrupts. */ > - virtqueue_disable_cb(vq); > - > - /* We were probably waiting for more output buffers. */ > - netif_wake_subqueue(vi->dev, vq2txq(vq)); > + virtqueue_disable_cb(sq->vq); > + napi_schedule(&sq->napi); > } > > static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx...

...f_tx_start_queue(txq); > + > + return packets; > +} > + > static void skb_xmit_done(struct virtqueue *vq) > { > struct virtnet_info *vi = vq->vdev->priv; > + struct send_queue *sq = &vi->sq[vq2txq(vq)]; > > - /* Suppress further interrupts. */ > - virtqueue_disable_cb(vq); > - > - /* We were probably waiting for more output buffers. */ > - netif_wake_subqueue(vi->dev, vq2txq(vq)); > + virtqueue_disable_cb(sq->vq); > + napi_schedule(&sq->napi); > } > > static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx...

...MAX_SKB_FRAGS); > > capacity = 10; //########## test code : force not to call netif_start_queue > > if (capacity >= 2+MAX_SKB_FRAGS) { > netif_start_queue(dev); > virtqueue_disable_cb(vi->svq); > } else { > //########## OTOH if often enter this branch tx queue maybe stopped. > } and changed it here so it won't restart queue if host consumed all buffers. unsurprisingly this makes driver not work. > > } &g...

...MAX_SKB_FRAGS); > > capacity = 10; //########## test code : force not to call netif_start_queue > > if (capacity >= 2+MAX_SKB_FRAGS) { > netif_start_queue(dev); > virtqueue_disable_cb(vi->svq); > } else { > //########## OTOH if often enter this branch tx queue maybe stopped. > } and changed it here so it won't restart queue if host consumed all buffers. unsurprisingly this makes driver not work. > > } &g...

...------ include/linux/virtio.h | 4 ++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/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 ret...