[PATCH v3 59/68] accel/tcg: Expose vcpu_[un]register() for RR

Philippe Mathieu-Daudé posted 68 patches 7 months, 1 week ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Reinoud Zandijk <reinoud@netbsd.org>, Sunil Muthuswamy <sunilmut@microsoft.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Alexander Graf <agraf@csgraf.de>
There is a newer version of this series
[PATCH v3 59/68] accel/tcg: Expose vcpu_[un]register() for RR
Posted by Philippe Mathieu-Daudé 7 months, 1 week ago
Allocate ForceRcuNotifier on the Heap.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tcg-accel-ops-icount.h |  4 ++--
 include/hw/core/cpu.h            |  2 ++
 accel/tcg/tcg-accel-ops-icount.c |  8 +++----
 accel/tcg/tcg-accel-ops-rr.c     | 36 +++++++++++++++++++++++---------
 4 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/accel/tcg/tcg-accel-ops-icount.h b/accel/tcg/tcg-accel-ops-icount.h
index 16a301b6dc0..5f3ebea50ff 100644
--- a/accel/tcg/tcg-accel-ops-icount.h
+++ b/accel/tcg/tcg-accel-ops-icount.h
@@ -11,8 +11,8 @@
 #define TCG_ACCEL_OPS_ICOUNT_H
 
 void icount_handle_deadline(void);
-void icount_prepare_for_run(CPUState *cpu, int64_t cpu_budget);
-int64_t icount_percpu_budget(int cpu_count);
+void icount_prepare_for_run(CPUState *cpu);
+void icount_update_percpu_budget(CPUState *cpu, int cpu_count);
 void icount_process_data(CPUState *cpu);
 
 void icount_handle_interrupt(CPUState *cpu, int mask);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 726427449da..952e44587b3 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -498,6 +498,8 @@ struct CPUState {
     int singlestep_enabled;
     int64_t icount_budget;
     int64_t icount_extra;
+    int64_t cpu_budget; /* FIXME TCG specific */
+
     uint64_t random_seed;
     sigjmp_buf jmp_env;
 
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c
index d0f7b410fab..ae1297ff7f3 100644
--- a/accel/tcg/tcg-accel-ops-icount.c
+++ b/accel/tcg/tcg-accel-ops-icount.c
@@ -90,7 +90,7 @@ void icount_handle_deadline(void)
 }
 
 /* Distribute the budget evenly across all CPUs */
-int64_t icount_percpu_budget(int cpu_count)
+void icount_update_percpu_budget(CPUState *cpu, int cpu_count)
 {
     int64_t limit = icount_get_limit();
     int64_t timeslice = limit / cpu_count;
@@ -99,10 +99,10 @@ int64_t icount_percpu_budget(int cpu_count)
         timeslice = limit;
     }
 
-    return timeslice;
+    cpu->cpu_budget = timeslice;
 }
 
-void icount_prepare_for_run(CPUState *cpu, int64_t cpu_budget)
+void icount_prepare_for_run(CPUState *cpu)
 {
     int insns_left;
 
@@ -116,7 +116,7 @@ void icount_prepare_for_run(CPUState *cpu, int64_t cpu_budget)
 
     replay_mutex_lock();
 
-    cpu->icount_budget = MIN(icount_get_limit(), cpu_budget);
+    cpu->icount_budget = MIN(icount_get_limit(), cpu->cpu_budget);
     insns_left = MIN(0xffff, cpu->icount_budget);
     cpu->neg.icount_decr.u16.low = insns_left;
     cpu->icount_extra = cpu->icount_budget - insns_left;
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 57a4bcab203..f5af7818d51 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -169,6 +169,27 @@ static int rr_cpu_count(void)
     return cpu_count;
 }
 
+static void *rr_vcpu_register(CPUState *cpu)
+{
+    Notifier *force_rcu = g_new(Notifier, 1);
+
+    assert(tcg_enabled());
+    force_rcu->notify = rr_force_rcu;
+    rcu_add_force_rcu_notifier(force_rcu);
+    tcg_register_thread();
+
+    return force_rcu;
+}
+
+static void rr_vcpu_unregister(CPUState *cpu, void *opaque)
+{
+    Notifier *force_rcu = opaque;
+
+    rcu_remove_force_rcu_notifier(force_rcu);
+
+    g_free(force_rcu);
+}
+
 /*
  * 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
@@ -179,14 +200,11 @@ static int rr_cpu_count(void)
 
 static void *rr_cpu_thread_fn(void *arg)
 {
-    Notifier force_rcu;
+    Notifier *force_rcu;
     CPUState *cpu = arg;
 
-    assert(tcg_enabled());
     rcu_register_thread();
-    force_rcu.notify = rr_force_rcu;
-    rcu_add_force_rcu_notifier(&force_rcu);
-    tcg_register_thread();
+    force_rcu = rr_vcpu_register(cpu);
 
     bql_lock();
     qemu_thread_get_self(cpu->thread);
@@ -217,9 +235,6 @@ static void *rr_cpu_thread_fn(void *arg)
     cpu->exit_request = 1;
 
     while (1) {
-        /* Only used for icount_enabled() */
-        int64_t cpu_budget = 0;
-
         bql_unlock();
         replay_mutex_lock();
         bql_lock();
@@ -235,7 +250,7 @@ static void *rr_cpu_thread_fn(void *arg)
              */
             icount_handle_deadline();
 
-            cpu_budget = icount_percpu_budget(cpu_count);
+            icount_update_percpu_budget(cpu, cpu_count);
         }
 
         replay_mutex_unlock();
@@ -258,7 +273,7 @@ static void *rr_cpu_thread_fn(void *arg)
 
                 bql_unlock();
                 if (icount_enabled()) {
-                    icount_prepare_for_run(cpu, cpu_budget);
+                    icount_prepare_for_run(cpu);
                 }
                 r = tcg_cpu_exec(cpu);
                 if (icount_enabled()) {
@@ -304,6 +319,7 @@ static void *rr_cpu_thread_fn(void *arg)
         rr_deal_with_unplugged_cpus();
     }
 
+    rr_vcpu_unregister(cpu, force_rcu);
     rcu_unregister_thread();
 
     g_assert_not_reached();
-- 
2.49.0


Re: [PATCH v3 59/68] accel/tcg: Expose vcpu_[un]register() for RR
Posted by Richard Henderson 7 months, 1 week ago
On 7/1/25 08:40, Philippe Mathieu-Daudé wrote:
> Allocate ForceRcuNotifier on the Heap.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/tcg-accel-ops-icount.h |  4 ++--
>   include/hw/core/cpu.h            |  2 ++
>   accel/tcg/tcg-accel-ops-icount.c |  8 +++----
>   accel/tcg/tcg-accel-ops-rr.c     | 36 +++++++++++++++++++++++---------
>   4 files changed, 34 insertions(+), 16 deletions(-)

Again, motivation?

As before, the structure is still not accessible from anywhere outside of the function, 
and has the same lifetime as the function.

Also, this is doing two things:

> +++ b/include/hw/core/cpu.h
> @@ -498,6 +498,8 @@ struct CPUState {
>      int singlestep_enabled;
>      int64_t icount_budget;
>      int64_t icount_extra;
> +    int64_t cpu_budget; /* FIXME TCG specific */

also moving cpu_budget.


r~