Re: [kvm-devel] [PATCH] qemu-kvm: Consolidate kvm_eat_signals

[Anthony L. <an...@co...>]

Avi Kivity wrote:
> Jan Kiszka wrote:
>   
>>> Given that with the iothread we spend very little time processing
>>> signals in vcpu threads, maybe it's better to drop the loop completely. 
>>> The common case is zero or one pending signals.  The uncommon case of
>>> two or more pending signals will be handled by the KVM_RUN ioctl
>>> returning immediately with -EINTR (i.e. in the outer loop).
>>>
>>>     
>>>       
>> You mean
>>
>> static void kvm_main_loop_wait(CPUState *env, int timeout)
>> {
>>     pthread_mutex_unlock(&qemu_mutex);
>>     kvm_eat_signal(env, timeout);
>>     pthread_mutex_lock(&qemu_mutex);
>>     cpu_single_env = env;
>>
>>     vcpu_info[env->cpu_index].signalled = 0;
>> }
>>
>> ?
>>   
>>     
>
> Yes.  The loop was a (perhaps premature) optimization that is now 
> totally unnecessary, unless I'm missing something quite large.
>   

It used to be that kvm_eat_signal() selected after consuming as many 
signals as possible while only sleeping once.  That's why there's a 
combination of sleeping and polling.

Now the VCPU threads never select so the whole loop can be simplified to 
a single sigtimedwait() that always blocks.

In reality, I don't think sigtimedwait() is really needed/useful for 
VCPUs anymore.  We only use it to catch SIG_IPI and we only use SIG_IPI 
to break out of sleeping.  I don't see any reason why we couldn't switch 
over to using a file descriptor for notification (or a pthread 
condition).  In the very least, we could just select() on nothing and 
allow SIG_IPI to break us out of the select.

Regards,

Anthony Liguori

> Oh.  There used to be a bug where we didn't check for a pending signal 
> before the first guest entry, so this would add a lot of latency 
> (effectively making the bug window much larger).  That was only closed 
> in 2.6.24 (by 7e66f350).
>
>