Re: [PATCH] interrupt preemption support

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Gregory Haskins wrote:
> This patch adds baseline support for interrupt preemption.  This lets one context (virtual SMP vcpu, PV driver, etc) preempt another vcpu which is currently in guest context when an interrupt is posted.  The patch consists of the following:
>
> 1) Re-worked the vcpu mutex with a new "MPL" construct.  This allows updates to the vcpu structure even if the vcpu is currently executing in guest mode.  The previous code held a mutex during guest execution.
>
>   

Whoa there.  kvm can't just add new locking constructs to the kernel.  
It has to be added to the kernel _first_, you need to justify it with 
more than just kvm as a user, show correctness, performance, and 
scalability.

Once it's accepted, kvm can use it.

Formal issues aside, why is this different from taking nested locks?  
The paravirt network code congealing in Dor's repo has a spinlock 
protecting the interrupt bits.  The main execution path takes both the 
vcpu mutex and the irq lock (when necessary), other paths take just the 
irq lock.  This has the added advantage of not requiring a mutex to 
inject an interrupt, which is often necessary from (host) irq context.


> 2) Exposed the previously static kvm_vcpu_ioctl_interrupt() as kvm_vcpu_interrupt so that other mechanisms (PV drivers) can issue interrupts just as the ioctl interface can
>   

It's not enough to issue an interrupt, there is a whole dance involved 
in the guest side to ack it.  You need to go through the apic, which is 
currently emulated in userspace.  We may push it to the kernel later.

>
> Index: kvm-12/kernel/kvm.h
>   

Please base things off trunk.  For kernel code, off the git repository, 
not the bundled kernel module (which is mangled by the release process 
in order to accommodate older kernels).

>  
> +typedef enum
> +{
> +    KVM_MPL_FREE,
> +    KVM_MPL_VMCS,
> +    KVM_MPL_VCPU,
> +    KVM_MPL_FULL = KVM_MPL_VCPU /* Must be equal to the last entry */ 
> +}kvm_mpl_locktypes_t;
>   

Either you or your mailer mangle whitespace horribly.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV