diff mbox series

[28/35] exec: access cpu->interrupt_request with atomics

Message ID 20180917163103.6113-29-cota@braap.org
State New
Headers show
Series exec: drop BQL from interrupt handling | expand

Commit Message

Emilio Cota Sept. 17, 2018, 4:30 p.m. UTC 
From: Paolo Bonzini <pbonzini@redhat.com>

Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 accel/tcg/cpu-exec.c      | 6 +++---
 accel/tcg/tcg-all.c       | 3 +--
 accel/tcg/translate-all.c | 2 +-
 qom/cpu.c                 | 6 +++---
 4 files changed, 8 insertions(+), 9 deletions(-)

Comments

Richard Henderson Sept. 18, 2018, 9:07 p.m. UTC | #1 
On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  accel/tcg/cpu-exec.c      | 6 +++---
>  accel/tcg/tcg-all.c       | 3 +--
>  accel/tcg/translate-all.c | 2 +-
>  qom/cpu.c                 | 6 +++---
>  4 files changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Paolo Bonzini Sept. 19, 2018, 5:02 p.m. UTC | #2 
On 18/09/2018 23:07, Richard Henderson wrote:
> On 9/17/18 9:30 AM, Emilio G. Cota wrote:
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>> Cc: Richard Henderson <rth@twiddle.net>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Emilio G. Cota <cota@braap.org>
>> ---
>>  accel/tcg/cpu-exec.c      | 6 +++---
>>  accel/tcg/tcg-all.c       | 3 +--
>>  accel/tcg/translate-all.c | 2 +-
>>  qom/cpu.c                 | 6 +++---
>>  4 files changed, 8 insertions(+), 9 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Richard, are you going to send a pull request from this?

Paolo
Richard Henderson Sept. 19, 2018, 6:18 p.m. UTC | #3 
On 9/19/18 10:02 AM, Paolo Bonzini wrote:
> On 18/09/2018 23:07, Richard Henderson wrote:
>> On 9/17/18 9:30 AM, Emilio G. Cota wrote:
>>> From: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>> Cc: Richard Henderson <rth@twiddle.net>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Emilio G. Cota <cota@braap.org>
>>> ---
>>>  accel/tcg/cpu-exec.c      | 6 +++---
>>>  accel/tcg/tcg-all.c       | 3 +--
>>>  accel/tcg/translate-all.c | 2 +-
>>>  qom/cpu.c                 | 6 +++---
>>>  4 files changed, 8 insertions(+), 9 deletions(-)
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Richard, are you going to send a pull request from this?

I could, though I have not queued it yet.
I'll not do anything until I'm back from Linaro Connect.


r~
Emilio Cota Sept. 19, 2018, 6:57 p.m. UTC | #4 
On Wed, Sep 19, 2018 at 11:18:48 -0700, Richard Henderson wrote:
> On 9/19/18 10:02 AM, Paolo Bonzini wrote:
> > On 18/09/2018 23:07, Richard Henderson wrote:
> >> On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> >>> From: Paolo Bonzini <pbonzini@redhat.com>
> >>>
> >>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> >>> Cc: Richard Henderson <rth@twiddle.net>
> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >>> Signed-off-by: Emilio G. Cota <cota@braap.org>
> >>> ---
> >>>  accel/tcg/cpu-exec.c      | 6 +++---
> >>>  accel/tcg/tcg-all.c       | 3 +--
> >>>  accel/tcg/translate-all.c | 2 +-
> >>>  qom/cpu.c                 | 6 +++---
> >>>  4 files changed, 8 insertions(+), 9 deletions(-)
> >>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > 
> > Richard, are you going to send a pull request from this?
> 
> I could, though I have not queued it yet.
> I'll not do anything until I'm back from Linaro Connect.

FWIW, I'm keeping a v2 branch here:

  https://github.com/cota/qemu/tree/cpu-lock-v2

I'm rebasing it with edits responding to feedback
as well as R-b tags as they trickle in.

		E.
Emilio Cota Sept. 25, 2018, 7:40 p.m. UTC | #5 
On Wed, Sep 19, 2018 at 11:18:48 -0700, Richard Henderson wrote:
> On 9/19/18 10:02 AM, Paolo Bonzini wrote:
> > On 18/09/2018 23:07, Richard Henderson wrote:
> >> On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> >>> From: Paolo Bonzini <pbonzini@redhat.com>
> >>>
> >>> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> >>> Cc: Richard Henderson <rth@twiddle.net>
> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >>> Signed-off-by: Emilio G. Cota <cota@braap.org>
> >>> ---
> >>>  accel/tcg/cpu-exec.c      | 6 +++---
> >>>  accel/tcg/tcg-all.c       | 3 +--
> >>>  accel/tcg/translate-all.c | 2 +-
> >>>  qom/cpu.c                 | 6 +++---
> >>>  4 files changed, 8 insertions(+), 9 deletions(-)
> >>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > 
> > Richard, are you going to send a pull request from this?
> 
> I could, though I have not queued it yet.
> I'll not do anything until I'm back from Linaro Connect.

Please do not queue this series; I am experiencing
hangs with large x86_64 guests (> 16 cores).

Once I get to the bottom of it, I'll send a v2.

Thanks,

		Emilio
diff mbox series

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 7ca00725ec..2383763f9b 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -426,7 +426,7 @@  static inline bool cpu_handle_halt(CPUState *cpu)
 {
     if (cpu->halted) {
 #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
-        if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
+        if ((atomic_read(&cpu->interrupt_request) & CPU_INTERRUPT_POLL)
             && replay_interrupt()) {
             X86CPU *x86_cpu = X86_CPU(cpu);
             qemu_mutex_lock_iothread();
@@ -527,7 +527,7 @@  static inline bool cpu_handle_interrupt(CPUState *cpu,
     if (unlikely(atomic_read(&cpu->interrupt_request))) {
         int interrupt_request;
         qemu_mutex_lock_iothread();
-        interrupt_request = cpu->interrupt_request;
+        interrupt_request = atomic_read(&cpu->interrupt_request);
         if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
             /* Mask out external interrupts for this step. */
             interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
@@ -579,7 +579,7 @@  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;
+            interrupt_request = atomic_read(&cpu->interrupt_request);
         }
         if (interrupt_request & CPU_INTERRUPT_EXITTB) {
             cpu_reset_interrupt(cpu, CPU_INTERRUPT_EXITTB);
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 3d25bdcc17..69ad44bd54 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -41,8 +41,7 @@  static void tcg_handle_interrupt(CPUState *cpu, int mask)
     int old_mask;
     g_assert(qemu_mutex_iothread_locked());
 
-    old_mask = cpu->interrupt_request;
-    cpu->interrupt_request |= mask;
+    old_mask = atomic_fetch_or(&cpu->interrupt_request, mask);
 
     /*
      * If called from iothread context, wake the target cpu in
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f7784bbc2d..364757c677 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2351,7 +2351,7 @@  void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
 void cpu_interrupt(CPUState *cpu, int mask)
 {
     g_assert(qemu_mutex_iothread_locked());
-    cpu->interrupt_request |= mask;
+    atomic_or(&cpu->interrupt_request, mask);
     atomic_set(&cpu->icount_decr.u16.high, -1);
 }
 
diff --git a/qom/cpu.c b/qom/cpu.c
index 20ad54d43f..e2dfbde7c4 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -103,7 +103,7 @@  void cpu_reset_interrupt(CPUState *cpu, int mask)
     if (need_lock) {
         qemu_mutex_lock_iothread();
     }
-    cpu->interrupt_request &= ~mask;
+    atomic_and(&cpu->interrupt_request, ~mask);
     if (need_lock) {
         qemu_mutex_unlock_iothread();
     }
@@ -261,7 +261,7 @@  static void cpu_common_reset(CPUState *cpu)
         log_cpu_state(cpu, cc->reset_dump_flags);
     }
 
-    cpu->interrupt_request = 0;
+    atomic_set(&cpu->interrupt_request, 0);
     cpu->halted = 0;
     cpu->mem_io_pc = 0;
     cpu->mem_io_vaddr = 0;
@@ -395,7 +395,7 @@  static vaddr cpu_adjust_watchpoint_address(CPUState *cpu, vaddr addr, int len)
 
 static void generic_handle_interrupt(CPUState *cpu, int mask)
 {
-    cpu->interrupt_request |= mask;
+    atomic_or(&cpu->interrupt_request, mask);
 
     if (!qemu_cpu_is_self(cpu)) {
         qemu_cpu_kick(cpu);