search for: enable_cb

Hello Rusty, seems that we still have a problem with virtio_net and the enable_cb callback. During a long running network stress tests with virtio and got the following oops: ------------[ cut here ]------------ kernel BUG at drivers/virtio/virtio_ring.c:230! illegal operation: 0001 [#1] SMP Modules linked in: CPU: 0 Not tainted 2.6.26-rc2-kvm-00436-gc94c08b-dirty #34 Process...

Hello Rusty, seems that we still have a problem with virtio_net and the enable_cb callback. During a long running network stress tests with virtio and got the following oops: ------------[ cut here ]------------ kernel BUG at drivers/virtio/virtio_ring.c:230! illegal operation: 0001 [#1] SMP Modules linked in: CPU: 0 Not tainted 2.6.26-rc2-kvm-00436-gc94c08b-dirty #34 Process...

...x6c [<000000000010ab08>] do_extint+0xb8/0xf0 [<0000000000110716>] ext_no_vtime+0x16/0x1a [<0000000000107e72>] cpu_idle+0x1c2/0x1e0 The problem can be triggered with a high amount of host->guest traffic. I think its the following race: poll says netif_rx_complete poll calls enable_cb enable_cb opens the interrupt mask a new packet comes, an interrupt is triggered----\ enable_cb sees that there is more work | enable_cb disables the interrupt | . V . interrupt is delivered...

...x6c [<000000000010ab08>] do_extint+0xb8/0xf0 [<0000000000110716>] ext_no_vtime+0x16/0x1a [<0000000000107e72>] cpu_idle+0x1c2/0x1e0 The problem can be triggered with a high amount of host->guest traffic. I think its the following race: poll says netif_rx_complete poll calls enable_cb enable_cb opens the interrupt mask a new packet comes, an interrupt is triggered----\ enable_cb sees that there is more work | enable_cb disables the interrupt | . V . interrupt is delivered...

...), 1 deletions(-) diff --git a/hack-module.awk b/hack-module.awk index 580090b..bd63fb9 100644 --- a/hack-module.awk +++ b/hack-module.awk @@ -45,8 +45,8 @@ print " return 1;"; print " }"; print ""; - print " no_work = vi->rvq->vq_ops->enable_cb(vi->rvq);"; print " netif_rx_complete(vi->dev);"; + print " no_work = vi->rvq->vq_ops->enable_cb(vi->rvq);"; print ""; print " if (!no_work && netif_rx_reschedule(vi->dev, received)) {"; print " s...

...blk.c +++ b/drivers/block/virtio_blk.c @@ -69,6 +69,8 @@ static void blk_done(struct virtqueue *v /* In case queue is stopped waiting for more buffers. */ blk_start_queue(vblk->disk->queue); spin_unlock_irqrestore(&vblk->lock, flags); + + vq->vq_ops->enable_cb(vq); } static bool do_req(struct request_queue *q, struct virtio_blk *vblk, Since this does not check the return status from enable_cb, it seems we could loose an interrupt if it arrives between poll and callback enable? Same might apply to other devices. Thanks, -- MST

...blk.c +++ b/drivers/block/virtio_blk.c @@ -69,6 +69,8 @@ static void blk_done(struct virtqueue *v /* In case queue is stopped waiting for more buffers. */ blk_start_queue(vblk->disk->queue); spin_unlock_irqrestore(&vblk->lock, flags); + + vq->vq_ops->enable_cb(vq); } static bool do_req(struct request_queue *q, struct virtio_blk *vblk, Since this does not check the return status from enable_cb, it seems we could loose an interrupt if it arrives between poll and callback enable? Same might apply to other devices. Thanks, -- MST

...l in add_buff function: When VRING_USED_F_NO_NOTIFY is set, the host does not kick the guest when packets were transmitted, as a result the guest runs out of tx buffers sometimes. This is fine but the problem lies when add_buf fails, it called notify and the host sends all the pending tx pkts. When enable_cb was called, more_used(vq) returned false so eventually the skb was dropped. Moving notify from add_buf to enable_cb fixes this flow problem. The tx performance boosted from 220Mbps to 850Mbps. Signed-off-by: Dor Laor <dor.laor@qumranet.com> --- drivers/virtio/virtio_ring.c | 3 +-- 1 fi...

...l in add_buff function: When VRING_USED_F_NO_NOTIFY is set, the host does not kick the guest when packets were transmitted, as a result the guest runs out of tx buffers sometimes. This is fine but the problem lies when add_buf fails, it called notify and the host sends all the pending tx pkts. When enable_cb was called, more_used(vq) returned false so eventually the skb was dropped. Moving notify from add_buf to enable_cb fixes this flow problem. The tx performance boosted from 220Mbps to 850Mbps. Signed-off-by: Dor Laor <dor.laor@qumranet.com> --- drivers/virtio/virtio_ring.c | 3 +-- 1 fi...

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead. Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential

virtqueue ops were introduced in the hope that we'll have multiple implementations besides virtio_ring, but none have surfaced so far, and given that existing virtio ring is deployed in production we are likely stuck with it now, so this layer just adds complexity and overhead. Further, the need to pass vq twice to each call (as in dev->vq->vq_ops->kick(dev->vq) ) adds potential

This adds a way to check ring empty state after enable_cb outside any locks. Will be used by virtio_net. Note: there's room for more optimization: caller is likely to have a memory barrier already, which means we might be able to get rid of a barrier here. Deferring this optimization until we do some benchmarking. Signed-off-by: Michael S. Tsirkin...

This adds a way to check ring empty state after enable_cb outside any locks. Will be used by virtio_net. Note: there's room for more optimization: caller is likely to have a memory barrier already, which means we might be able to get rid of a barrier here. Deferring this optimization until we do some benchmarking. Signed-off-by: Michael S. Tsirkin...

Hi Rusty, I hit a bug in virtio_ring.c:218 when I was stressing virtio_net using kvm with -smp 4. static void vring_disable_cb(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); START_USE(vq); --> BUG_ON(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;

Hi Rusty, I hit a bug in virtio_ring.c:218 when I was stressing virtio_net using kvm with -smp 4. static void vring_disable_cb(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); START_USE(vq); --> BUG_ON(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;

...vi->svq, sg, num, 0, skb); - if (err) { - pr_debug("%s: virtio not prepared to send\n", dev->name); - netif_stop_queue(dev); - /* Activate callback for using skbs: if this returns false it - * means some were used in the meantime. */ - if (unlikely(!vi->svq->vq_ops->enable_cb(vi->svq))) { - vi->svq->vq_ops->disable_cb(vi->svq); - netif_start_queue(dev); - goto again; - } - __skb_unlink(skb, &vi->send); + /* If we has a buffer left over from last time, send it now. */ + if (vi->last_xmit_skb) { + if (xmit_skb(vi, vi->last_xmit_skb) !=...

...vi->svq, sg, num, 0, skb); - if (err) { - pr_debug("%s: virtio not prepared to send\n", dev->name); - netif_stop_queue(dev); - /* Activate callback for using skbs: if this returns false it - * means some were used in the meantime. */ - if (unlikely(!vi->svq->vq_ops->enable_cb(vi->svq))) { - vi->svq->vq_ops->disable_cb(vi->svq); - netif_start_queue(dev); - goto again; - } - __skb_unlink(skb, &vi->send); + /* If we has a buffer left over from last time, send it now. */ + if (vi->last_xmit_skb) { + if (xmit_skb(vi, vi->last_xmit_skb) !=...

Rusty, Anthony, Dor, I need your brain power :-) On smp guests I have seen a problem with virtio (the version in curent Avi's git) which do not occur on single processor guests: kernel BUG at /space/kvm/drivers/virtio/virtio_ring.c:228! illegal operation: 0001 [#1] Modules linked in: ipv6 CPU: 2 Not tainted Process swapper (pid: 0, task: 000000000f83e038, ksp: 000000000f877d70) Krnl

Rusty, Anthony, Dor, I need your brain power :-) On smp guests I have seen a problem with virtio (the version in curent Avi's git) which do not occur on single processor guests: kernel BUG at /space/kvm/drivers/virtio/virtio_ring.c:228! illegal operation: 0001 [#1] Modules linked in: ipv6 CPU: 2 Not tainted Process swapper (pid: 0, task: 000000000f83e038, ksp: 000000000f877d70) Krnl

...14 05:34 PM, Michael S. Tsirkin wrote: >>> On Wed, Oct 15, 2014 at 03:25:25PM +0800, Jason Wang wrote: >>>> This patch checks the new event idx to make sure used event idx never >>>> goes back. This is used to synchronize the calls between >>>> virtqueue_enable_cb_delayed() and virtqueue_enable_cb(). >>>> >>>> Cc: Rusty Russell <rusty at rustcorp.com.au> >>>> Cc: Michael S. Tsirkin <mst at redhat.com> >>>> Signed-off-by: Jason Wang <jasowang at redhat.com> >>> the implication being th...