Marcelo Tosatti wrote:
> Hi Anthony,
>
> How is -no-kvm-irqchip working with the patch?
>
Seems to work fine. What is your expectation?
> On Tue, Apr 29, 2008 at 09:28:14AM -0500, Anthony Liguori wrote:
>
>> This patch eliminates the use of sigtimedwait() in the IO thread. To avoid
>> the
>> signal/select race condition, we use a pipe that we write to in the signal
>> handlers. This was suggested by Rusty and seems to work well.
>>
>> +static int kvm_eat_signal(CPUState *env, int timeout)
>> {
>> struct timespec ts;
>> int r, e, ret = 0;
>> siginfo_t siginfo;
>> + sigset_t waitset;
>>
>> + sigemptyset(&waitset);
>> + sigaddset(&waitset, SIG_IPI);
>> ts.tv_sec = timeout / 1000;
>> ts.tv_nsec = (timeout % 1000) * 1000000;
>> - r = sigtimedwait(&waitset->sigset, &siginfo, &ts);
>> + qemu_kvm_unlock();
>> + r = sigtimedwait(&waitset, &siginfo, &ts);
>> + qemu_kvm_lock(env);
>> + cpu_single_env = env;
>>
>
> This assignment seems redundant now.
>
Yeah, I have a bigger patch which eliminates all of the explicit
assignments to cpu_single_env.
>>
>> @@ -263,12 +238,8 @@ static void pause_all_threads(void)
>> vcpu_info[i].stop = 1;
>> pthread_kill(vcpu_info[i].thread, SIG_IPI);
>>
>
> Make sure the IO thread has SIG_IPI blocked (those are for APIC vcpu
> initialization only).
>
Just so I'm clear, there's really no harm in not blocking SIG_IPI
because it would just be ignored by the IO thread (since the SIG_IPI
handler is a nop). But yeah, we should explicitly block it.
>> +static void sig_aio_fd_read(void *opaque)
>> +{
>> + int signum;
>> + ssize_t len;
>> +
>> + do {
>> + len = read(kvm_sigfd[0], &signum, sizeof(signum));
>> + } while (len == -1 && errno == EINTR);
>>
>
> What is the reason for this loop instead of a straight read?
>
> Its alright to be interrupted by a signal.
>
Just general habit with QEMU.
>> + signal(SIGUSR1, sig_aio_handler);
>> + signal(SIGUSR2, sig_aio_handler);
>> + signal(SIGALRM, sig_aio_handler);
>> + signal(SIGIO, sig_aio_handler);
>> +
>> + if (pipe(kvm_sigfd) == -1)
>> + abort();
>>
>
> perror() would be nice.
>
Yeah, everything needs proper error handling.
>> - kvm_eat_signal(&io_signal_table, NULL, 1000);
>> pthread_mutex_lock(&qemu_mutex);
>> - cpu_single_env = NULL;
>> - main_loop_wait(0);
>> + main_loop_wait(10);
>>
>
> Increase that 1000 or something. Will make it easier to spot bugs.
>
I have actually and it does introduce some bugs. I'm not entirely clear
what is causing them though.
Regards,
Anthony Liguori
> Similarly in qemu_kvm_aio_wait().
>
>
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
