Re: [PATCH 3/8] KVM: Adds ability to preempt an executing VCPU

["Gregory Haskins" <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>]

>>> On Mon, May 14, 2007 at 11:45 AM, in message <46488426.8090705-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote: 
> Gregory Haskins wrote:
>>>> index 059f074..0f6cc32 100644
>>>> ---   a/drivers/kvm/kvm.h
>>>> +++ b/drivers/kvm/kvm.h
>>>> @@ -  329,6 +329,8 @@ struct kvm_vcpu_irq {
>>>>  	struct kvm_irqdevice dev;
>>>>  	int                  pending;
>>>>  	int                  deferred;
>>>> +	struct task_struct  *task;
>>>> +	int                  guest_mode;
>>>>   
>>>>       
>>> -  >guest_mode can be folded into -  >task, by specifying that -  >task != 
>>> NULL is equivalent to -  >guest_mode != 0.  This will make the rest of the 
>>> code easier to read.
>>>     
>>
>> The problem with doing it this way is that its no longer possible to detect 
> the optimizing condition of "irq.task != current" when injecting interrupts.  
> This means that userspace will be inadvertently sending itself a signal every 
> time it injects interrupts, which IMHO is undesirable.
>>
>>   
> 
> I meant keeping - >task and dropping - >guest_mode.  Or did I 
> misunderstand something?

Its possible that I am actually misunderstanding you instead, but from my perspective those two variables are tracking orthogonal state.  irq.task is keeping track of the thread that is running the CPU.  This will tend to get set once (on the first entry to kvm_run() and stay unchanged for the duration of the VM.  irq.guest_mode, on the other hand, will track whether the vcpu is in (or near) guest mode (to switch between direct_ipi and eventfd wakeup methods).

I like having both states tracked, because it allows me to optimize the vcpu interrupt if the context of the injection is the same as the execution.  E.g. if the single QEMU thread calls KVM_RUN and then KVM_INTERRUPT, I can skip sending an eventfd because I know the irq.task == current and its pointless.  

(Note that in the original designs, irq.task was also used to designate a target for send_sig.  Perhaps it is no longer logical to have this scoped to the vcpu.irq structure anymore?  E.g. should I make it vcpu.task?)


> 
>>>>  
>>>> +	vcpu-  >irq.task = current;
>>>> +	smp_wmb();
>>>> +
>>>>   
>>>>       
>>> This is best moved where -  >guest_mode is set.
>>>     
>>
>> I can do this, but its common to all platforms so I figured it was best to 
> be out here?
>>
>>   
> 
> Well, it scatters the logic.  If we can merge guest_mode and task it's 
> moot anyway.

Sounds reasonable.  If you convince me to condense this it goes away outright, otherwise I will move it together. ;)





-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/