[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread |
Date: |
Mon, 21 Jun 2010 22:58:32 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
Jan Kiszka wrote:
> Marcelo Tosatti wrote:
>> Clear exit_request when iothread grabs the global lock.
>>
>> Signed-off-by: Marcelo Tosatti <address@hidden>
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 026980a..74cb973 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -236,10 +236,8 @@ int cpu_exec(CPUState *env1)
>> asm("");
>> env = env1;
>>
>> - if (exit_request) {
>> + if (exit_request)
>> env->exit_request = 1;
>> - exit_request = 0;
>> - }
>
> Coding style...
>
>>
>> #if defined(TARGET_I386)
>> if (!kvm_enabled()) {
>> diff --git a/cpus.c b/cpus.c
>> index fcd0f09..ef1ab22 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -598,6 +598,7 @@ void qemu_mutex_lock_iothread(void)
>> }
>> qemu_mutex_unlock(&qemu_fair_mutex);
>> }
>> + exit_request = 0;
>> }
>>
>> void qemu_mutex_unlock_iothread(void)
>>
>>
>
> I looked into this a bit as well, and that's what I also have in my
> queue.
>
> But things are still widely broken: pause_all_vcpus and run_on_cpu as
> there is no guarantee that all VCPUs regularly call into
> qemu_wait_io_event. Also breakpoints don't work, not only in SMP mode.
>
> The former issue is mostly fine with this patch:
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 026980a..9f4191d 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -236,9 +236,8 @@ int cpu_exec(CPUState *env1)
> asm("");
> env = env1;
>
> - if (exit_request) {
> + if (unlikely(exit_request)) {
> env->exit_request = 1;
> - exit_request = 0;
> }
>
> #if defined(TARGET_I386)
> diff --git a/cpus.c b/cpus.c
> index fcd0f09..2ce839d 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -39,7 +39,6 @@
> #define SIG_IPI SIGUSR1
> #endif
>
> -static CPUState *cur_cpu;
> static CPUState *next_cpu;
>
> /***********************************************************/
> @@ -331,6 +330,7 @@ int qemu_init_main_loop(void)
> return ret;
>
> qemu_cond_init(&qemu_pause_cond);
> + qemu_cond_init(&qemu_system_cond);
> qemu_mutex_init(&qemu_fair_mutex);
> qemu_mutex_init(&qemu_global_mutex);
> qemu_mutex_lock(&qemu_global_mutex);
> @@ -401,10 +401,12 @@ static void qemu_wait_io_event_common(CPUState *env)
> flush_queued_work(env);
> }
>
> -static void qemu_wait_io_event(CPUState *env)
> +static void qemu_tcg_wait_io_event(void)
> {
> + CPUState *env;
> +
> while (!tcg_has_work())
> - qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
> + qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
>
> qemu_mutex_unlock(&qemu_global_mutex);
>
> @@ -417,7 +419,10 @@ static void qemu_wait_io_event(CPUState *env)
> qemu_mutex_unlock(&qemu_fair_mutex);
>
> qemu_mutex_lock(&qemu_global_mutex);
> - qemu_wait_io_event_common(env);
> +
> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
> + qemu_wait_io_event_common(env);
> + }
> }
>
> static void qemu_kvm_eat_signal(CPUState *env, int timeout)
> @@ -502,7 +507,7 @@ static void *tcg_cpu_thread_fn(void *arg)
>
> while (1) {
> tcg_cpu_exec();
> - qemu_wait_io_event(cur_cpu);
> + qemu_tcg_wait_io_event();
> }
>
> return NULL;
> @@ -768,11 +773,11 @@ bool tcg_cpu_exec(void)
>
> if (next_cpu == NULL)
> next_cpu = first_cpu;
> - for (; next_cpu != NULL; next_cpu = next_cpu->next_cpu) {
> - CPUState *env = cur_cpu = next_cpu;
> + for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu)
> {
> + CPUState *env = next_cpu;
>
> qemu_clock_enable(vm_clock,
> - (cur_cpu->singlestep_enabled & SSTEP_NOTIMER) ==
> 0);
> + (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
>
> if (qemu_alarm_pending())
> break;
> @@ -787,6 +792,7 @@ bool tcg_cpu_exec(void)
> break;
> }
> }
> + exit_request = 0;
> return tcg_has_work();
> }
>
>
> I haven't looked into the breakpoint bug yet as performance (likely
> VCPU scheduling) in iothread mode is still suffering compared to classic
> mode, specifically when you go up with the number of VCPUs (try -smp 8).
Hmm, retesting this, I cannot reproduce the performance regression
anymore. Likely I confused something or had instrumentations applied.
Locking up still works "reliably". :(
Jan
> And there is some race that cause a lock up in qemu_mutex_lock_iothread
> after a while (the cpu_unlink_tb seems to race with the linking - just a
> guess so far).
>
> Anyway, all this is getting terribly complex, hard to grasp, and maybe
> therefore fragile as well. Specifically the SMP round-robin scheduling
> looks like it requires a redesign for iothread mode (my feeling is it
> only works by chance ATM). Part of the issue may not be new, but could
> have been shadowed by the frequently interrupting I/O load when using a
> single thread. The rest seems to lack a bit a user base...
>
> Jan
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] smp broken in tcg mode with --enable-io-thread?, Jan Kiszka, 2010/06/18
- Re: [Qemu-devel] smp broken in tcg mode with --enable-io-thread?, Anthony Liguori, 2010/06/18
- [Qemu-devel] [PATCH] fix smp with tcg mode and --enable-io-thread, Marcelo Tosatti, 2010/06/21
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Jan Kiszka, 2010/06/21
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread,
Jan Kiszka <=
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Marcelo Tosatti, 2010/06/21
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Jan Kiszka, 2010/06/22
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Jan Kiszka, 2010/06/23
- Re: [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Anthony Liguori, 2010/06/23
- [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Jan Kiszka, 2010/06/21
- Re: [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Alexander Graf, 2010/06/21
- Re: [Qemu-devel] Re: [PATCH] fix smp with tcg mode and --enable-io-thread, Jan Kiszka, 2010/06/22