Re: [Qemu-devel] [RFC v1 10/11] tcg: grab iothread lock in cpu-exec interrupt handling

[Alex Bennée]

Paolo Bonzini <address@hidden> writes:

> On 18/03/2016 17:18, Alex Bennée wrote:
>> From: "Emilio G. Cota" <address@hidden>
>>
>> Signed-off-by: Emilio G. Cota <address@hidden>
>> Signed-off-by: Alex Bennée <address@hidden>
>>
>> ---
>> v1 (ajb)
>>  - pulled from emilio/mttcg series
>> ---
>>  cpu-exec.c        | 33 +++++++++++++++++++++++++++------
>>  include/qom/cpu.h |  1 +
>>  2 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 76891fd..6acaf25 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -351,6 +351,29 @@ static void cpu_handle_debug_exception(CPUState *cpu)
>>      cc->debug_excp_handler(cpu);
>>  }
>>
>> +#ifdef CONFIG_SOFTMMU
>> +static inline void cpu_exit_loop_lock(CPUState *cpu)
>> +{
>> +    qemu_mutex_lock_iothread();
>> +    cpu->cpu_loop_exit_locked = true;
>> +}
>> +
>> +static inline void cpu_exit_loop_lock_reset(CPUState *cpu)
>> +{
>> +    if (cpu->cpu_loop_exit_locked) {
>> +        cpu->cpu_loop_exit_locked = false;
>> +        qemu_mutex_unlock_iothread();
>> +    }
>> +}
>
> In the meanwhile we got qemu_mutex_iothread_locked(), so these two
> inlines are not necessary anymore.

Good point, I'll fix that up as I squash them.

>
> Paolo
>
>> +#else
>> +static inline void cpu_exit_loop_lock(CPUState *cpu)
>> +{ }
>> +
>> +static inline void cpu_exit_loop_lock_reset(CPUState *cpu)
>> +{ }
>> +#endif
>> +
>>  /* main execution loop */
>>
>>  int cpu_exec(CPUState *cpu)
>> @@ -450,12 +473,7 @@ int cpu_exec(CPUState *cpu)
>>              for(;;) {
>>                  interrupt_request = cpu->interrupt_request;
>>                  if (unlikely(interrupt_request)) {
>> -                    /* FIXME: this needs to take the iothread lock.
>> -                     * For this we need to find all places in
>> -                     * cc->cpu_exec_interrupt that can call cpu_loop_exit,
>> -                     * and call qemu_unlock_iothread_mutex() there.  Else,
>> -                     * add a flag telling cpu_loop_exit() to unlock it.
>> -                     */
>> +                    cpu_exit_loop_lock(cpu);
>>
>>                      if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
>>                          /* Mask out external interrupts for this step. */
>> @@ -510,6 +528,8 @@ int cpu_exec(CPUState *cpu)
>>                          next_tb = 0;
>>                      }
>>
>> +                    cpu_exit_loop_lock_reset(cpu);
>> +
>>                  }
>>                  if (unlikely(cpu->exit_request
>>                               || replay_has_interrupt())) {
>> @@ -630,6 +650,7 @@ int cpu_exec(CPUState *cpu)
>>
>>              cpu->can_do_io = 1;
>>              tb_lock_reset();
>> +            cpu_exit_loop_lock_reset(cpu);
>>          }
>>      } /* for(;;) */
>>
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index d6cb7b8..954d97d 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -295,6 +295,7 @@ struct CPUState {
>>      bool crash_occurred;
>>      bool exit_request;
>>      bool tb_invalidated_flag;
>> +    bool cpu_loop_exit_locked;
>>      uint32_t interrupt_request;
>>      int singlestep_enabled;
>>      int64_t icount_extra;
>>


--
Alex Bennée