Before: root@ubuntu:~# time tcpblast -o -s 65536 -c 16k 192.168.2.1:9999 > /dev/null real 0m13.802s user 0m2.260s sys 0m11.540s root@ubuntu:~# cat /proc/interrupts CPU0 1: 1332 lguest-virtio virtio0 2: 246200 lguest-virtio virtio1 3: 1 lguest-virtio virtio1 4: 3 lguest-virtio virtio2 5: 0 lguest-virtio virtio2 net virtqueue net_input: add_used = 259297 trigger_irq = 257405 (1156 suppressed, 0 forced empty, 0 unnecessary, 10049 already set, 36606 disabled, 248 blocked) add_used per trigger = 1 to 36 (average 1.01) wait_for_vq_desc = 259298 (0 trigger_irq, 0 race, 0 eventfd, 0 no-avail net virtqueue net_output: add_used = 753673 trigger_irq = 23115 (17037 suppressed, 0 forced empty, 6077 unnecessary, 0 already set, 0 disabled, 0 blocked) add_used per trigger = 0 to 1500 (average 32.61) wait_for_vq_desc = 753674 (23115 trigger_irq, 0 race, 23115 eventfd, 6076 no-avail After: root@ubuntu:~# time tcpblast -o -s 65536 -c 16k 192.168.2.1:9999 > /dev/null real 0m14.810s user 0m2.170s sys 0m12.640s root@ubuntu:~# cat /proc/interrupts CPU0 1: 1309 lguest-virtio virtio0 2: 290266 lguest-virtio virtio1 3: 1 lguest-virtio virtio1 4: 20 lguest-virtio virtio2 5: 0 lguest-virtio virtio2 net virtqueue net_input (irq 2): add_used = 302911 trigger_irq = 302911 (12645 suppressed, 0 forced empty, 0 unnecessary, 0 already set, 45631 disabled, 0 blocked) add_used per trigger = 1 to 1 (average 1.00) wait_for_vq_desc = 302912 (0 trigger_irq, 0 race, 0 eventfd, 0 no-avail net virtqueue net_output (irq 3): add_used = 753672 trigger_irq = 4962 (4949 suppressed, 0 forced empty, 12 unnecessary, 0 already set, 0 disabled, 0 blocked) add_used per trigger = 0 to 1838 (average 151.89) wait_for_vq_desc = 753673 (4962 trigger_irq, 0 race, 4962 eventfd, 11 no-avail --- Documentation/lguest/Makefile | 3 - Documentation/lguest/lguest.c | 32 +++++++++----- drivers/lguest/hypercalls.c | 8 +++ drivers/lguest/interrupts_and_traps.c | 10 ++++ drivers/lguest/lg.h | 2 drivers/lguest/lguest_device.c | 6 ++ drivers/net/virtio_net.c | 32 +++++++++++--- drivers/virtio/virtio_ring.c | 73 +++++++++++++++++++++++++++++----- include/linux/virtio_ring.h | 23 +++++++++- 9 files changed, 158 insertions(+), 31 deletions(-) diff --git a/Documentation/lguest/Makefile b/Documentation/lguest/Makefile --- a/Documentation/lguest/Makefile +++ b/Documentation/lguest/Makefile @@ -1,5 +1,6 @@ # This creates the demonstration utility "lguest" which runs a Linux guest. -CFLAGS:=-Wall -Wmissing-declarations -Wmissing-prototypes -O3 -I../../include -I../../arch/x86/include -U_FORTIFY_SOURCE +#CFLAGS:=-Wall -Wmissing-declarations -Wmissing-prototypes -O3 -I../../include -I../../arch/x86/include -U_FORTIFY_SOURCE +CFLAGS:=-Wall -Wmissing-declarations -Wmissing-prototypes -g -I../../include -I../../arch/x86/include -U_FORTIFY_SOURCE LDLIBS:=-lz all: lguest diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -580,11 +580,21 @@ static void trigger_irq(struct virtqueue vq->pending_used = 0; /* If they don't want an interrupt, don't send one. */ - if (vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT) { + if (vring_avail_tailer(&vq->vring)->cons_wkcount + == vring_used_tailer(&vq->vring)->prod_wkcount) { (*vq->irq_suppressed)++; return; } + vring_used_tailer(&vq->vring)->prod_wkcount++; + + if (vring_avail_tailer(&vq->vring)->cons_wkcount + != vring_used_tailer(&vq->vring)->prod_wkcount) + fprintf(stderr, "%s: counts = %u/%u\n", + vq->name, + vring_used_tailer(&vq->vring)->prod_wkcount, + vring_avail_tailer(&vq->vring)->cons_wkcount); + /* Send the Guest an interrupt tell them we used something up. */ if (write(lguest_fd, buf, sizeof(buf)) != 0 && errno != EEXIST) err(1, "Triggering irq %i", vq->config.irq); @@ -611,22 +621,19 @@ static unsigned wait_for_vq_desc(struct trigger_irq(vq); /* OK, now we need to know about added descriptors. */ - vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY; + vring_used_tailer(&vq->vring)->cons_wkcount + = vring_avail_tailer(&vq->vring)->prod_wkcount + 1; /* They could have slipped one in as we were doing that: make * sure it's written, then check again. */ mb(); - if (last_avail != vq->vring.avail->idx) { - vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY; + if (last_avail != vq->vring.avail->idx) + /* We'll get a spurious notify now. */ break; - } /* Nothing new? Wait for eventfd to tell us they refilled. */ if (read(vq->eventfd, &event, sizeof(event)) != sizeof(event)) errx(1, "Event read failed?"); - - /* We don't need to be notified again. */ - vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY; } /* Check it isn't doing very strange things with descriptor numbers. */ @@ -1125,13 +1132,13 @@ static void dump_info(int signal) for (dev = devices.dev; dev; dev = dev->next) { for (vq = dev->vq; vq; vq = vq->next) { - p += sprintf(p, "%s virtqueue %s:\n" + p += sprintf(p, "%s virtqueue %s (irq %u):\n" "\tpending = %u\n" "\tavail = %u/%u\n" "\tused = %u/%u\n" "\tused flags (notify suppress) = %u\n" "\tavail dflags (irq suppress) = %u\n", - dev->name, vq->name, + dev->name, vq->name, vq->config.irq, vq->pending_used, lg_last_avail(vq), vq->vring.avail->idx, vring_last_used(&vq->vring), @@ -1148,6 +1155,7 @@ static void start_device(struct device * unsigned int i; struct virtqueue *vq; + fprintf(stderr, "Device %s OK: offered\n", dev->name); verbose("Device %s OK: offered", dev->name); for (i = 0; i < dev->feature_len; i++) verbose(" %02x", get_feature_bits(dev)[i]); @@ -1307,6 +1315,7 @@ static void _add_virtqueue(struct device /* Initialize the vring. */ vring_init(&vq->vring, num_descs, p, LGUEST_VRING_ALIGN); + vring_used_tailer(&vq->vring)->cons_wkcount = 1; /* Append virtqueue to this device's descriptor. We use * device_config() to get the end of the device's current virtqueues; @@ -1340,10 +1349,11 @@ static void add_feature(struct device *d features[bit / CHAR_BIT] |= (1 << (bit % CHAR_BIT)); } -/* This sets the PUBLISH_INDICES feature for every device. */ +/* This sets the PUBLISH_INDICES and VRING_WKCOUNT features for every device. */ static void finalize_features(struct device *dev) { add_feature(dev, VIRTIO_RING_F_PUBLISH_INDICES); + add_feature(dev, VIRTIO_RING_F_WKCOUNT); } /* This routine sets the configuration fields for an existing device's diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c --- a/drivers/lguest/hypercalls.c +++ b/drivers/lguest/hypercalls.c @@ -41,6 +41,14 @@ static void do_hcall(struct lg_cpu *cpu, case LHCALL_SEND_INTERRUPTS: /* This call does nothing too, but by breaking out of the Guest * it makes us process any pending interrupts. */ + if (cpu->lg->hcall_stats[cpu->hcall->arg0] % 100 == 0) { + unsigned int irq; + bool more = false; + irq = interrupt_pending(cpu, &more); + printk("SEND_INTERRUPTS: was %u, %u pending%s\n", + cpu->last_irq, irq, more ? " (and more)" : ""); + } + break; case LHCALL_LGUEST_INIT: /* You can't get here unless you're already initialized. Don't diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c --- a/drivers/lguest/interrupts_and_traps.c +++ b/drivers/lguest/interrupts_and_traps.c @@ -238,6 +238,15 @@ void try_deliver_interrupt(struct lg_cpu if (get_user(irq_enabled, &cpu->lg->lguest_data->irq_enabled)) irq_enabled = 0; if (!irq_enabled) { + static int counter; + + if (counter++ % 100 == 0) { + unsigned int x; + + get_user(x, &cpu->lg->lguest_data->irq_pending); + printk("irqs disabled for irq %u after irq %u, eip=%#lx pending was %#x\n", + irq, cpu->last_irq, cpu->regs->eip, x); + } /* Make sure they know an IRQ is pending. */ put_user(X86_EFLAGS_IF, &cpu->lg->lguest_data->irq_pending); @@ -260,6 +269,7 @@ void try_deliver_interrupt(struct lg_cpu * flag to say whether this interrupt pushes an error code onto * the stack as well: virtual interrupts never do. */ set_guest_interrupt(cpu, idt->a, idt->b, false); + cpu->last_irq = irq; } /* Every time we deliver an interrupt, we update the timestamp in the diff --git a/drivers/lguest/lg.h b/drivers/lguest/lg.h --- a/drivers/lguest/lg.h +++ b/drivers/lguest/lg.h @@ -74,6 +74,8 @@ struct lg_cpu { /* Did the Guest tell us to halt? */ int halted; + unsigned int last_irq; + /* Pending virtual interrupts */ DECLARE_BITMAP(irqs_pending, LGUEST_IRQS); diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -90,10 +90,14 @@ static u32 lg_get_features(struct virtio struct lguest_device_desc *desc = to_lgdev(vdev)->desc; u8 *in_features = lg_features(desc); + printk("lg_get_features for %s\n", dev_name(&vdev->dev)); /* We do this the slow but generic way. */ for (i = 0; i < min(desc->feature_len * 8, 32); i++) - if (in_features[i / 8] & (1 << (i % 8))) + if (in_features[i / 8] & (1 << (i % 8))) { + printk("%s: feature %i is set\n", + dev_name(&vdev->dev), i); features |= (1 << i); + } return features; } diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -374,7 +374,10 @@ static void try_fill_recv(struct virtnet static void skb_recv_done(struct virtqueue *rvq) { struct virtnet_info *vi = rvq->vdev->priv; - napi_schedule(&vi->napi); + if (napi_schedule_prep(&vi->napi)) + __napi_schedule(&vi->napi); + else + printk(KERN_WARNING "skb_recv_done: no napi_schedule_prep\n"); } static int virtnet_poll(struct napi_struct *napi, int budget) @@ -382,6 +385,7 @@ static int virtnet_poll(struct napi_stru struct virtnet_info *vi = container_of(napi, struct virtnet_info, napi); struct sk_buff *skb = NULL; unsigned int len, received = 0; + bool debug = false; again: while (received < budget && @@ -398,16 +402,30 @@ again: try_fill_recv(vi); /* Out of packets? */ + if (debug) + printk("Received %u of %u\n", received, budget); if (received < budget) { napi_complete(napi); - if (unlikely(!vi->rvq->vq_ops->enable_cb(vi->rvq)) - && napi_schedule_prep(napi)) { - vi->rvq->vq_ops->disable_cb(vi->rvq); - __napi_schedule(napi); - goto again; - } + if (unlikely(!vi->rvq->vq_ops->enable_cb(vi->rvq))) { + if (debug) + printk("Then enable_cb failed\n"); + if (napi_schedule_prep(napi)) { + printk(KERN_WARNING "virtnet_poll: race\n"); + vi->rvq->vq_ops->disable_cb(vi->rvq); + __napi_schedule(napi); + debug = true; + printk(KERN_WARNING "virtnet_poll: received %u\n", received); + goto again; + } + printk(KERN_WARNING "virtnet_poll: enable fail\n"); + } else + if (debug) + printk("Then enable_cb succeeded\n"); + } + if (debug) + printk("virtnet_poll: %u received\n", received); return received; } diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -21,6 +21,7 @@ #include #include +#define DEBUG #ifdef DEBUG /* For development, we want to crash whenever the ring is screwed. */ #define BAD_RING(_vq, fmt, args...) \ @@ -68,6 +69,13 @@ struct vring_virtqueue /* Have we had an under-threshold descriptor since last. */ unsigned int descs_since_indirect_review; + /* Use flags as wakeup counter. */ + bool wake_counter; + /* Last value of interrupt send counter. */ + unsigned last_wkcount; + /* Do we need to ack? */ + unsigned long needs_ack; + /* Number of free buffers */ unsigned int num_free; /* Head of free buffer list. */ @@ -221,7 +229,7 @@ static int vring_add_buf(struct virtqueu /* FIXME: for historical reasons, we force a notify here if * there are outgoing parts to the buffer. Presumably the * host should service the ring ASAP. */ - if (out) { + if (out && !vq->wake_counter) { vq->notify(&vq->vq); vq->other_notify++; } @@ -267,6 +275,27 @@ add_head: return 0; } +#include +static bool should_notify(struct vring_virtqueue *vq) +{ + char buf[128]; + + if (!vq->wake_counter) + return !(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY); + + sprintf(buf, "%s: us = %u, them = %u\n", + dev_name(&vq->vq.vdev->dev), + vring_avail_tailer(&vq->vring)->prod_wkcount, + vring_used_tailer(&vq->vring)->cons_wkcount); +// kvm_hypercall1(LHCALL_NOTIFY, __pa(buf)); + if (vring_avail_tailer(&vq->vring)->prod_wkcount + == vring_used_tailer(&vq->vring)->cons_wkcount) + return false; + + vring_avail_tailer(&vq->vring)->prod_wkcount++; + return true; +} + static void vring_kick(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); @@ -282,7 +311,7 @@ static void vring_kick(struct virtqueue /* Need to update avail index before checking if we should notify */ mb(); - if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY)) + if (should_notify(vq)) /* Prod other side to tell it about changes. */ vq->notify(&vq->vq); else @@ -369,7 +398,10 @@ static void vring_disable_cb(struct virt { struct vring_virtqueue *vq = to_vvq(_vq); - vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + if (!vq->wake_counter) + vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + + /* With wake_counter, we don't actually allow disabling again. */ } static bool vring_enable_cb(struct virtqueue *_vq) @@ -380,7 +412,11 @@ static bool vring_enable_cb(struct virtq /* We optimistically turn back on interrupts, then check if there was * more to do. */ - vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; + if (!vq->wake_counter) + vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; + else + vring_avail_tailer(&vq->vring)->cons_wkcount + = vq->last_wkcount+1; mb(); if (unlikely(more_used(vq))) { END_USE(vq); @@ -395,9 +431,18 @@ irqreturn_t vring_interrupt(int irq, voi { struct vring_virtqueue *vq = to_vvq(_vq); - if (!more_used(vq)) { - pr_debug("virtqueue interrupt with no work for %p\n", vq); - return IRQ_NONE; + if (!vq->wake_counter) { + if (!more_used(vq)) { + pr_debug("virtqueue interrupt with no work for %p\n", vq); + return IRQ_NONE; + } + } else { + u16 wkcount = vring_used_tailer(&vq->vring)->prod_wkcount; + if (wkcount == vq->last_wkcount) { + printk("spurious virtqueue interrupt for %p\n", vq); + return IRQ_NONE; + } + vq->last_wkcount = wkcount; } if (unlikely(vq->broken)) @@ -450,6 +495,9 @@ struct virtqueue *vring_new_virtqueue(un vq->broken = false; vq->num_added = 0; list_add_tail(&vq->vq.list, &vdev->vqs); + vq->wake_counter = test_bit(VIRTIO_RING_F_WKCOUNT, vdev->features); + vq->last_wkcount = 256; + vq->needs_ack = 0; #ifdef DEBUG vq->in_use = false; #endif @@ -465,8 +513,13 @@ struct virtqueue *vring_new_virtqueue(un virtio_has_feature(vdev, VIRTIO_RING_F_PUBLISH_INDICES); /* No callback? Tell other side not to bother us. */ - if (!callback) - vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + if (vq->wake_counter) { + if (callback) + vring_avail_tailer(&vq->vring)->cons_wkcount = 1; + } else { + if (!callback) + vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + } /* Put everything in free lists. */ vq->num_free = num; @@ -496,6 +549,8 @@ void vring_transport_features(struct vir break; case VIRTIO_RING_F_PUBLISH_INDICES: break; + case VIRTIO_RING_F_WKCOUNT: + break; default: /* We don't understand this bit. */ clear_bit(i, vdev->features); diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -32,6 +32,9 @@ /* We publish our last-seen used index at the end of the avail ring. */ #define VIRTIO_RING_F_PUBLISH_INDICES 29 +/* Do we use the use a wakeup count? */ +#define VIRTIO_RING_F_WKCOUNT 30 + /* Virtio ring descriptors: 16 bytes. These can chain together via "next". */ struct vring_desc { @@ -68,6 +71,12 @@ struct vring_used struct vring_used_elem ring[]; }; +struct vring_tailer { + __u16 last_idx; + __u8 prod_wkcount; + __u8 cons_wkcount; +}; + struct vring { unsigned int num; @@ -91,6 +100,8 @@ struct vring { * __u16 avail_idx; * __u16 available[num]; * __u16 last_used_idx; + * __u8 prod_wkcount; + * __u8 cons_wkcount; * * // Padding to the next align boundary. * char pad[]; @@ -100,6 +111,8 @@ struct vring { * __u16 used_idx; * struct vring_used_elem used[num]; * __u16 last_avail_idx; + * __u8 prod_wkcount; + * __u8 cons_wkcount; * }; */ static inline void vring_init(struct vring *vr, unsigned int num, void *p, @@ -119,10 +132,16 @@ static inline unsigned vring_size(unsign + sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num + 2; } +/* For backwards compatibility, various fields are at the end. */ +#define vring_used_tailer(vr) \ + ((struct vring_tailer *)&(vr)->used->ring[(vr)->num]) +#define vring_avail_tailer(vr) \ + ((struct vring_tailer *)&(vr)->avail->ring[(vr)->num]) + /* We publish the last-seen used index at the end of the available ring, and * vice-versa. These are at the end for backwards compatibility. */ -#define vring_last_used(vr) ((vr)->avail->ring[(vr)->num]) -#define vring_last_avail(vr) (*(__u16 *)&(vr)->used->ring[(vr)->num]) +#define vring_last_used(vr) (vring_used_tailer(vr)->last_idx) +#define vring_last_avail(vr) (vring_avail_tailer(vr)->last_idx) #ifdef __KERNEL__ #include