lguest: improve interrupt handling, speed up stream networking
authorRusty Russell <rusty@rustcorp.com.au>
Sat, 13 Jun 2009 04:27:02 +0000 (22:27 -0600)
committerRusty Russell <rusty@rustcorp.com.au>
Fri, 12 Jun 2009 12:57:03 +0000 (22:27 +0930)
lguest never checked for pending interrupts when enabling interrupts, and
things still worked.  However, it makes a significant difference to TCP
performance, so it's time we fixed it by introducing a pending_irq flag
and checking it on irq_restore and irq_enable.

These two routines are now too big to patch into the 8/10 bytes
patch space, so we drop that code.

Note: The high latency on interrupt delivery had a very curious
effect: once everything else was optimized, networking without GSO was
faster than networking with GSO, since more interrupts were sent and
hence a greater chance of one getting through to the Guest!

Note2: (Almost) Closing the same loophole for iret doesn't have any
measurable effect, so I'm leaving that patch for the moment.

Before:
1GB tcpblast Guest->Host: 30.7 seconds
1GB tcpblast Guest->Host (no GSO): 76.0 seconds

After:
1GB tcpblast Guest->Host: 6.8 seconds
1GB tcpblast Guest->Host (no GSO): 27.8 seconds

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
arch/x86/include/asm/lguest_hcall.h
arch/x86/lguest/boot.c
arch/x86/lguest/i386_head.S
drivers/lguest/core.c
drivers/lguest/hypercalls.c
drivers/lguest/interrupts_and_traps.c
drivers/lguest/lg.h
include/linux/lguest.h

index faae199..f9a9f78 100644 (file)
@@ -17,6 +17,7 @@
 #define LHCALL_LOAD_TLS                16
 #define LHCALL_NOTIFY          17
 #define LHCALL_LOAD_GDT_ENTRY  18
+#define LHCALL_SEND_INTERRUPTS 19
 
 #define LGUEST_TRAP_ENTRY 0x1F
 
index 2392a7a..37b8c1d 100644 (file)
@@ -205,6 +205,12 @@ PV_CALLEE_SAVE_REGS_THUNK(save_fl);
 static void restore_fl(unsigned long flags)
 {
        lguest_data.irq_enabled = flags;
+       mb();
+       /* Null hcall forces interrupt delivery now, if irq_pending is
+        * set to X86_EFLAGS_IF (ie. an interrupt is pending, and flags
+        * enables interrupts. */
+       if (flags & lguest_data.irq_pending)
+               kvm_hypercall0(LHCALL_SEND_INTERRUPTS);
 }
 PV_CALLEE_SAVE_REGS_THUNK(restore_fl);
 
@@ -219,6 +225,11 @@ PV_CALLEE_SAVE_REGS_THUNK(irq_disable);
 static void irq_enable(void)
 {
        lguest_data.irq_enabled = X86_EFLAGS_IF;
+       mb();
+       /* Null hcall forces interrupt delivery now. */
+       if (lguest_data.irq_pending)
+               kvm_hypercall0(LHCALL_SEND_INTERRUPTS);
+
 }
 PV_CALLEE_SAVE_REGS_THUNK(irq_enable);
 
@@ -972,10 +983,10 @@ static void lguest_restart(char *reason)
  *
  * Our current solution is to allow the paravirt back end to optionally patch
  * over the indirect calls to replace them with something more efficient.  We
- * patch the four most commonly called functions: disable interrupts, enable
- * interrupts, restore interrupts and save interrupts.  We usually have 6 or 10
- * bytes to patch into: the Guest versions of these operations are small enough
- * that we can fit comfortably.
+ * patch two of the simplest of the most commonly called functions: disable
+ * interrupts and save interrupts.  We usually have 6 or 10 bytes to patch
+ * into: the Guest versions of these operations are small enough that we can
+ * fit comfortably.
  *
  * First we need assembly templates of each of the patchable Guest operations,
  * and these are in i386_head.S. */
@@ -986,8 +997,6 @@ static const struct lguest_insns
        const char *start, *end;
 } lguest_insns[] = {
        [PARAVIRT_PATCH(pv_irq_ops.irq_disable)] = { lgstart_cli, lgend_cli },
-       [PARAVIRT_PATCH(pv_irq_ops.irq_enable)] = { lgstart_sti, lgend_sti },
-       [PARAVIRT_PATCH(pv_irq_ops.restore_fl)] = { lgstart_popf, lgend_popf },
        [PARAVIRT_PATCH(pv_irq_ops.save_fl)] = { lgstart_pushf, lgend_pushf },
 };
 
index f795419..3e0c554 100644 (file)
@@ -46,8 +46,6 @@ ENTRY(lguest_entry)
        .globl lgstart_##name; .globl lgend_##name
 
 LGUEST_PATCH(cli, movl $0, lguest_data+LGUEST_DATA_irq_enabled)
-LGUEST_PATCH(sti, movl $X86_EFLAGS_IF, lguest_data+LGUEST_DATA_irq_enabled)
-LGUEST_PATCH(popf, movl %eax, lguest_data+LGUEST_DATA_irq_enabled)
 LGUEST_PATCH(pushf, movl lguest_data+LGUEST_DATA_irq_enabled, %eax)
 /*:*/
 
index 8ca1def..03fbc88 100644 (file)
@@ -189,6 +189,7 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
        /* We stop running once the Guest is dead. */
        while (!cpu->lg->dead) {
                unsigned int irq;
+               bool more;
 
                /* First we run any hypercalls the Guest wants done. */
                if (cpu->hcall)
@@ -213,9 +214,9 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
                /* Check if there are any interrupts which can be delivered now:
                 * if so, this sets up the hander to be executed when we next
                 * run the Guest. */
-               irq = interrupt_pending(cpu);
+               irq = interrupt_pending(cpu, &more);
                if (irq < LGUEST_IRQS)
-                       try_deliver_interrupt(cpu, irq);
+                       try_deliver_interrupt(cpu, irq, more);
 
                /* All long-lived kernel loops need to check with this horrible
                 * thing called the freezer.  If the Host is trying to suspend,
@@ -233,7 +234,7 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
                        set_current_state(TASK_INTERRUPTIBLE);
                        /* Just before we sleep, make sure nothing snuck in
                         * which we should be doing. */
-                       if (interrupt_pending(cpu) < LGUEST_IRQS
+                       if (interrupt_pending(cpu, &more) < LGUEST_IRQS
                            || cpu->break_out)
                                set_current_state(TASK_RUNNING);
                        else
index 54d66f0..f252b71 100644 (file)
@@ -37,6 +37,10 @@ static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
                /* This call does nothing, except by breaking out of the Guest
                 * it makes us process all the asynchronous hypercalls. */
                break;
+       case LHCALL_SEND_INTERRUPTS:
+               /* This call does nothing too, but by breaking out of the Guest
+                * it makes us process any pending interrupts. */
+               break;
        case LHCALL_LGUEST_INIT:
                /* You can't get here unless you're already initialized.  Don't
                 * do that. */
index a8c966f..5a10754 100644 (file)
@@ -131,7 +131,7 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
  * interrupt_pending() returns the first pending interrupt which isn't blocked
  * by the Guest.  It is called before every entry to the Guest, and just before
  * we go to sleep when the Guest has halted itself. */
-unsigned int interrupt_pending(struct lg_cpu *cpu)
+unsigned int interrupt_pending(struct lg_cpu *cpu, bool *more)
 {
        unsigned int irq;
        DECLARE_BITMAP(blk, LGUEST_IRQS);
@@ -149,13 +149,14 @@ unsigned int interrupt_pending(struct lg_cpu *cpu)
 
        /* Find the first interrupt. */
        irq = find_first_bit(blk, LGUEST_IRQS);
+       *more = find_next_bit(blk, LGUEST_IRQS, irq+1);
 
        return irq;
 }
 
 /* This actually diverts the Guest to running an interrupt handler, once an
  * interrupt has been identified by interrupt_pending(). */
-void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq)
+void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more)
 {
        struct desc_struct *idt;
 
@@ -178,8 +179,12 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq)
                u32 irq_enabled;
                if (get_user(irq_enabled, &cpu->lg->lguest_data->irq_enabled))
                        irq_enabled = 0;
-               if (!irq_enabled)
+               if (!irq_enabled) {
+                       /* Make sure they know an IRQ is pending. */
+                       put_user(X86_EFLAGS_IF,
+                                &cpu->lg->lguest_data->irq_pending);
                        return;
+               }
        }
 
        /* Look at the IDT entry the Guest gave us for this interrupt.  The
@@ -202,6 +207,11 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq)
         * here is a compromise which means at least it gets updated every
         * timer interrupt. */
        write_timestamp(cpu);
+
+       /* If there are no other interrupts we want to deliver, clear
+        * the pending flag. */
+       if (!more)
+               put_user(0, &cpu->lg->lguest_data->irq_pending);
 }
 /*:*/
 
index 6743cf1..5738965 100644 (file)
@@ -139,8 +139,8 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user);
 #define pgd_pfn(x)     (pgd_val(x) >> PAGE_SHIFT)
 
 /* interrupts_and_traps.c: */
-unsigned int interrupt_pending(struct lg_cpu *cpu);
-void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq);
+unsigned int interrupt_pending(struct lg_cpu *cpu, bool *more);
+void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more);
 bool deliver_trap(struct lg_cpu *cpu, unsigned int num);
 void load_guest_idt_entry(struct lg_cpu *cpu, unsigned int i,
                          u32 low, u32 hi);
index 175e63f..7bc1440 100644 (file)
@@ -30,6 +30,10 @@ struct lguest_data
        /* Wallclock time set by the Host. */
        struct timespec time;
 
+       /* Interrupt pending set by the Host.  The Guest should do a hypercall
+        * if it re-enables interrupts and sees this set (to X86_EFLAGS_IF). */
+       int irq_pending;
+
        /* Async hypercall ring.  Instead of directly making hypercalls, we can
         * place them in here for processing the next time the Host wants.
         * This batching can be quite efficient. */