On 03/06/16 23:40, Alex Bennée wrote:
> The only remaining use of the global exit_request flag is now to ensure
> we exit the run_loop when we first start to process pending work. This
> is just as easily done by setting the first_cpu->exit_request flag.
>
> We lightly re-factor the main vCPU thread to ensure cpu->exit_requests
> cause us to exit the main loop and process any IO requests that might
> come along.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> ---
> cpu-exec-common.c | 2 --
> cpu-exec.c | 10 +++-------
> cpus.c | 35 +++++++++++++++++++++++------------
> include/exec/exec-all.h | 2 --
> 4 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/cpu-exec-common.c b/cpu-exec-common.c
> index f42d24a..1b4fb53 100644
> --- a/cpu-exec-common.c
> +++ b/cpu-exec-common.c
> @@ -23,8 +23,6 @@
> #include "exec/exec-all.h"
> #include "exec/memory-internal.h"
>
> -bool exit_request;
> -
> /* exit the current TB from a signal handler. The host registers are
> restored in a state compatible with the CPU emulator
> */
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 68e804b..1613c63 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -541,9 +541,8 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
> TranslationBlock *tb,
> /* Something asked us to stop executing
> * chained TBs; just continue round the main
> * loop. Whatever requested the exit will also
> - * have set something else (eg exit_request or
> - * interrupt_request) which we will handle
> - * next time around the loop. But we need to
> + * have set something else (eg interrupt_request) which we
> + * will handle next time around the loop. But we need to
> * ensure the tcg_exit_req read in generated code
> * comes before the next read of cpu->exit_request
> * or cpu->interrupt_request.
> @@ -594,15 +593,12 @@ int cpu_exec(CPUState *cpu)
> current_cpu = cpu;
>
> if (cpu_handle_halt(cpu)) {
> + cpu->exit_request = true;
Why do we need to do this here?
> return EXCP_HALTED;
> }
>
> rcu_read_lock();
>
> - if (unlikely(atomic_mb_read(&exit_request))) {
> - cpu->exit_request = 1;
> - }
> -
> cc->cpu_exec_enter(cpu);
>
> /* Calculate difference between guest clock and host clock.
> diff --git a/cpus.c b/cpus.c
> index a139ad3..a84f02c 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1169,6 +1169,18 @@ static int64_t tcg_get_icount_limit(void)
> }
> }
>
> +static void handle_icount_deadline(void)
> +{
> + if (use_icount) {
> + int64_t deadline =
> + qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
> +
> + if (deadline == 0) {
> + qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
> + }
> + }
> +}
> +
> static int tcg_cpu_exec(CPUState *cpu)
> {
> int ret;
> @@ -1276,11 +1288,11 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> timer_mod(kick_timer, TCG_KICK_FREQ);
> }
>
> - /* process any pending work */
> - atomic_mb_set(&exit_request, 1);
> -
> cpu = first_cpu;
>
> + /* process any pending work */
> + atomic_mb_set(&cpu->exit_request, 1);
Sometimes we use atomic_*() to operate on 'cpu->exit_request', sometimes
not. I am wondering if there are some rules about that?
Kind regards,
Sergey
> +
> while (1) {
> /* Account partial waits to QEMU_CLOCK_VIRTUAL. */
> qemu_account_warp_timer();
> @@ -1289,7 +1301,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> cpu = first_cpu;
> }
>
> - for (; cpu != NULL && !exit_request; cpu = CPU_NEXT(cpu)) {
> + while (cpu && !cpu->exit_request) {
> atomic_mb_set(&tcg_current_rr_cpu, cpu);
>
> qemu_clock_enable(QEMU_CLOCK_VIRTUAL,
> @@ -1303,22 +1315,21 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> }
> } else if (cpu->stop || cpu->stopped) {
> break;
> + } else if (cpu->exit_request) {
> + break;
> }
>
> + cpu = CPU_NEXT(cpu);
> } /* for cpu.. */
> /* Does not need atomic_mb_set because a spurious wakeup is okay. */
> atomic_set(&tcg_current_rr_cpu, NULL);
>
> - /* Pairs with smp_wmb in qemu_cpu_kick. */
> - atomic_mb_set(&exit_request, 0);
> + if (cpu && cpu->exit_request) {
> + atomic_mb_set(&cpu->exit_request, 0);
> + }
>
> - if (use_icount) {
> - int64_t deadline =
> qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
> + handle_icount_deadline();
>
> - if (deadline == 0) {
> - qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
> - }
> - }
> qemu_tcg_wait_io_event(QTAILQ_FIRST(&cpus));
> }
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 31f4c38..7919aac 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -405,6 +405,4 @@ bool memory_region_is_unassigned(MemoryRegion *mr);
> /* vl.c */
> extern int singlestep;
>
> -extern bool exit_request;
> -
> #endif
