Re: [PATCH 1/2] kvm tools: Respect ISR status in virtio header

[Anthony Liguori <anthony@codemonkey.ws>]

On 05/07/2011 09:02 AM, Ingo Molnar wrote:
> Well the optimization also avoids unnecessary VM exits (due to the injection,
> which interrupts a guest context immediately, even if it's running on another
> CPU), not just system calls - so it could be more expensive than a system call,
> right?

If there's IRQ is already pending, the syscall should be a nop.  If the 
IRQ is extraneous, then there's a pretty short window when the IRQ 
wouldn't already be pending.

I took a look at the current virtio code and noticed a few things that 
are sub-optimal:

1) No MSI.  MSI makes interrupts edge triggered and avoids the ISR read 
entirely.  The ISR read is actually pretty expensive.

2) The access pattern is basically:

while pending_requests:
    a = get_next_request();
    process_next_request(a);

But this is not the optimal pattern with virtio.  The optimal pattern is:

if pending_requests:
   disable_notifications();

again:
   while pending_requests:
     a = get_next_request();
     process_next_request();

   enable_notifications();
   if pending_requests:
      goto again;

You're currently taking exits way more than you should, particularly 
since you're using threads for processing the ring queues.

3) You aren't advertising VIRTIO_F_NOTIFY_ON_EMPTY.  Particularly with 
TX, it's advantageous to inject an IRQ even if they're disabled when 
there are no longer packets in the ring.

(2) and (1) are definitely the most important for performance.  (2) 
should be a pretty simple change and should have a significant impact.

Regards,

Anthony Liguori

>
> Thanks,
>
> 	Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html