Menu
▾
▴
kvm-devel
|
From: Jan K. <jan...@we...> - 2008-05-12 10:50:06
|
Having both kvm_eat_signal and kvm_eat_signals makes the code harder to
read. Moreover, given the single caller of kvm_eat_signals, there is no
real reason to keep it in a separate function.
Signed-off-by: Jan Kiszka <jan...@we...>
---
qemu/qemu-kvm.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -210,11 +210,12 @@ static int kvm_eat_signal(CPUState *env,
return ret;
}
-
-static void kvm_eat_signals(CPUState *env, int timeout)
+static void kvm_main_loop_wait(CPUState *env, int timeout)
{
int r = 0;
+ pthread_mutex_unlock(&qemu_mutex);
+
while (kvm_eat_signal(env, 0))
r = 1;
if (!r && timeout) {
@@ -223,14 +224,10 @@ static void kvm_eat_signals(CPUState *en
while (kvm_eat_signal(env, 0))
;
}
-}
-static void kvm_main_loop_wait(CPUState *env, int timeout)
-{
- pthread_mutex_unlock(&qemu_mutex);
- kvm_eat_signals(env, timeout);
pthread_mutex_lock(&qemu_mutex);
cpu_single_env = env;
+
vcpu_info[env->cpu_index].signalled = 0;
}
|
|
From: Avi K. <av...@qu...> - 2008-05-12 11:33:43
|
Jan Kiszka wrote: > Having both kvm_eat_signal and kvm_eat_signals makes the code harder to > read. Moreover, given the single caller of kvm_eat_signals, there is no > real reason to keep it in a separate function. > > 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). -- error compiling committee.c: too many arguments to function |
|
From: Jan K. <jan...@we...> - 2008-05-12 11:42:10
Attachments:
signature.asc
|
Avi Kivity wrote:
> Jan Kiszka wrote:
>> Having both kvm_eat_signal and kvm_eat_signals makes the code harder to
>> read. Moreover, given the single caller of kvm_eat_signals, there is no
>> real reason to keep it in a separate function.
>>
>>
>
> 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;
}
?
Jan
|
|
From: Avi K. <av...@qu...> - 2008-05-12 11:56:41
|
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.
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).
--
error compiling committee.c: too many arguments to function
|
|
From: Anthony L. <an...@co...> - 2008-05-12 14:31:47
|
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).
>
>
|
|
From: Avi K. <av...@qu...> - 2008-05-12 15:44:56
|
Anthony Liguori wrote: >> >> 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. > Yes. > 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). How would you stop a vcpu running in guest mode? > In the very least, we could just select() on nothing and allow SIG_IPI > to break us out of the select. sigtimedwait() (or just sigwait, now) doesn't require the signal to be delivered, so it's faster. If there's nothing to select, why call select()? -- error compiling committee.c: too many arguments to function |
|
From: Anthony L. <an...@co...> - 2008-05-12 15:11:58
|
Avi Kivity wrote: >> 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). > > How would you stop a vcpu running in guest mode? Yeah, I forgot about that. >> In the very least, we could just select() on nothing and allow >> SIG_IPI to break us out of the select. > > sigtimedwait() (or just sigwait, now) doesn't require the signal to be > delivered, so it's faster. Yeah, sigtimedwait() is probably the right thing to do since we have to use a signal for IPI. Regards, Anthony Liguori > If there's nothing to select, why call select()? > |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X