|
From: | Richard Henderson |
Subject: | Re: [PATCH v2] accel/tcg/tcg-accel-ops-rr: ensure fairness with icount |
Date: | Mon, 24 Apr 2023 14:06:18 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 |
On 4/24/23 12:29, Jamie Iles wrote:
+/* + * Calculate the number of CPUs that we will process in a single iteration of + * the main CPU thread loop so that we can fairly distribute the instruction + * count across CPUs. + * + * The CPU count is cached based on the CPU list generation ID to avoid + * iterating the list every time. + */ +static int rr_cpu_count(void) +{ + static unsigned int last_gen_id = ~0; + static int cpu_count; + CPUState *cpu; + + cpu_list_lock(); + if (cpu_list_generation_id_get() != last_gen_id) { + cpu_count = 0; + CPU_FOREACH(cpu) { + ++cpu_count; + } + last_gen_id = cpu_list_generation_id_get(); + } + cpu_list_unlock(); + + return cpu_count; +}
The read of cpu_count should be in the lock.(Ideally we'd expose QEMU_LOCK_GUARD(&qemu_cpu_list_lock) which would make the read+return trivial.)
/* * In the single-threaded case each vCPU is simulated in turn. If * there is more than a single vCPU we create a simple timer to kick @@ -185,6 +212,9 @@ static void *rr_cpu_thread_fn(void *arg) cpu->exit_request = 1;while (1) {+ int cpu_count = rr_cpu_count(); + int64_t cpu_budget = INT64_MAX; + qemu_mutex_unlock_iothread(); replay_mutex_lock(); qemu_mutex_lock_iothread(); @@ -197,6 +227,8 @@ static void *rr_cpu_thread_fn(void *arg) * waking up the I/O thread and waiting for completion. */ icount_handle_deadline(); + + cpu_budget = icount_percpu_budget(cpu_count);
I think you can move the call to rr_cpu_count() here, so that it only happens if icount is in use.
Why cpu_budget = INT64_MAX as opposed to 0 or 0xdeadbeef? It's not set or used except for icount_enabled.
r~
[Prev in Thread] | Current Thread | [Next in Thread] |