Skip to content

Commit

Permalink
lguest: improve interrupt handling, speed up stream networking
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Rusty Russell committed Jun 12, 2009
1 parent abd41f0 commit a32a881
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 16 deletions.
1 change: 1 addition & 0 deletions arch/x86/include/asm/lguest_hcall.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
21 changes: 15 additions & 6 deletions arch/x86/lguest/boot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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. */
Expand All @@ -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 },
};

Expand Down
2 changes: 0 additions & 2 deletions arch/x86/lguest/i386_head.S
Original file line number Diff line number Diff line change
Expand Up @@ -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)
/*:*/

Expand Down
7 changes: 4 additions & 3 deletions drivers/lguest/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions drivers/lguest/hypercalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
16 changes: 13 additions & 3 deletions drivers/lguest/interrupts_and_traps.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;

Expand All @@ -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
Expand All @@ -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);
}
/*:*/

Expand Down
4 changes: 2 additions & 2 deletions drivers/lguest/lg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions include/linux/lguest.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down

0 comments on commit a32a881

Please sign in to comment.