On Thu, Sep 03, 2009 at 02:01:26PM -0400, Glauber Costa wrote:
> Hi guys
>
> In this patch, I am attaching an early version of a new "on_vcpu" mechanism
> (after
> making it generic, I saw no reason to keep its name). It allows us to
> guarantee
> that a piece of code will be executed in a certain vcpu, indicated by a
> CPUState.
>
> I am sorry for the big patch, I just dumped what I had so we can have early
> directions.
> When it comes to submission state, I'll split it accordingly.
>
> As we discussed these days at qemu-devel, I am using pthread_set/get_specific
> for
> dealing with thread-local variables. Note that they are not used from signal
> handlers.
> A first optimization would be to use TLS variables where available.
>
> In vl.c, I am providing a version of queue_work for the IO-thread, and other
> for normal
> operation. The "normal" one should fix the problems Jan is having, since it
> does nothing
> more than just issuing the function we want to execute.
>
> The io-thread version is tested with both tcg and kvm, and works (to the
> extent they were
> working before, which in kvm case, is not much)
>
> Changes from v1:
> * Don't open the possibility of asynchronous calling queue_work, suggested by
> Avi "Peter Parker" Kivity
> * Use a local mutex, suggested by Paolo Bonzini
>
> Signed-off-by: Glauber Costa <glom...@redhat.com>
> ---
> cpu-all.h | 3 ++
> cpu-defs.h | 15 ++++++++++++
> exec.c | 1 +
> kvm-all.c | 58 +++++++++++++++++++---------------------------
> kvm.h | 7 +++++
> vl.c | 75
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 125 insertions(+), 34 deletions(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 1a6a812..529479e 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -763,6 +763,9 @@ extern CPUState *cpu_single_env;
> extern int64_t qemu_icount;
> extern int use_icount;
>
> +void qemu_queue_work(CPUState *env, void (*func)(void *data), void *data);
> +void qemu_flush_work(CPUState *env);
> +
> #define CPU_INTERRUPT_HARD 0x02 /* hardware interrupt pending */
> #define CPU_INTERRUPT_EXITTB 0x04 /* exit the current TB (use for x86 a20
> case) */
> #define CPU_INTERRUPT_TIMER 0x08 /* internal timer exception pending */
> @@ -3808,6 +3835,50 @@ void qemu_cpu_kick(void *_env)
> qemu_thread_signal(env->thread, SIGUSR1);
> }
>
> +void qemu_queue_work(CPUState *env, void (*func)(void *data), void *data)
> +{
> + QemuWorkItem wii;
> +
> + env->queued_total++;
> +
> + if (env == qemu_get_current_env()) {
> + env->queued_local++;
> + func(data);
> + return;
> + }
> +
> + wii.func = func;
> + wii.data = data;
> + qemu_mutex_lock(&env->queue_lock);
> + TAILQ_INSERT_TAIL(&env->queued_work, &wii, entry);
> + qemu_mutex_unlock(&env->queue_lock);
> +
> + qemu_thread_signal(env->thread, SIGUSR1);
> +
> + qemu_mutex_lock(&env->queue_lock);
> + while (!wii.done) {
> + qemu_cond_wait(&env->work_cond, &qemu_global_mutex);
> + }
> + qemu_mutex_unlock(&env->queue_lock);
How's qemu_flush_work supposed to execute if env->queue_lock is held
here?
qemu_cond_wait() should work with env->queue_lock, and qemu_global_mutex
should be dropped before waiting and reacquired on return.
