[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-riscv
Subject:    Re: [PATCH v2 7/7] target: Push BQL on ->cpu_exec_interrupt down into per-arch implementation
From:       Richard Henderson <richard.henderson () linaro ! org>
Date:       2020-08-31 22:11:48
Message-ID: 42dc8454-9835-fc52-a7e2-73835672f3f9 () linaro ! org
[Download RAW message or body]

On 8/19/20 11:28 AM, Robert Foley wrote:
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 499a8bdc5e..c4e54baa6f 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -599,7 +599,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>             True when it is, and we should restart on a new TB,
>             and via longjmp via cpu_loop_exit.  */
>          else {
> -            qemu_mutex_lock_iothread();
>              if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
>                  replay_interrupt();
>                  /*
> @@ -614,7 +613,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>              /* The target hook may have updated the 'cpu->interrupt_request';
>               * reload the 'interrupt_request' value */
>              interrupt_request = cpu_interrupt_request(cpu);
> -            qemu_mutex_unlock_iothread();
>          }
>          if (interrupt_request & CPU_INTERRUPT_EXITTB) {
>              cpu_reset_interrupt(cpu, CPU_INTERRUPT_EXITTB);

I think you need a change to patch 5, because this patch reduces the scope
further, around cpu_interrupt_request, etc.

I think patch 5 should have

+    bool success;
+    qemu_mutex_lock_iothread();
+    success = cc->cpu_exec_interrupt(cpu, interrupt_request);
+    qemu_mutex_unlok_iothread();
+    if (success) {
-    if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {

That way this patch maintains the scope of the lock.


r~


[prev in list] [next in list] [prev in thread] [next in thread]