Re: [PATCH] qemu-kvm: Consolidate kvm_eat_signals

Anthony Liguori
2008-05-12T14:31:46Z

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).