Re: [PATCH][RFC] Use pipe() to simulate signalfd()

[Marcelo Tosatti <mtosatti@redhat.com>]

Hi Anthony,

How is -no-kvm-irqchip working with the patch?

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.

>      if (r == -1 && (errno == EAGAIN || errno == EINTR) && !timeout)
>  	return 0;
>      e = errno;
> -    pthread_mutex_lock(&qemu_mutex);
>      if (env && vcpu)
>          cpu_single_env = vcpu->env;

And this one too.

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

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

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

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

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