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

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

Gregory Haskins wrote:
>>>> 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.  
>   

You can't rely on irq.task if !guest_mode.  Under the current design,
the task may have exited and you'd be dereferencing unallocated memory. 
While it won't oops or cause anything bad to happen (and current qemu
can't trigger this), it isn't nice.

Later we'll have vcpu and thread_info point to each other and then you
can do that kind of optimization.

Oh, and nobody said that the task waiting on the event is the same as
the task running the vcpu.

> (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?)
>   

I think so.



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


-------------------------------------------------------------------------
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/