[v1] accel: Only include qdev-realized vCPUs in global &cpus_queue

[RFC PATCH 3/9] cpus: Remove cpu from global queue after UNREALIZE completed

Posted by Philippe Mathieu-Daudé 2 months ago

Previous commit removed the restriction on completing the full QDev
UNREALIZE step before removing vCPUs from global queue, it is now
safe to call cpu_list_remove() after accel_cpu_common_unrealize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 cpu-target.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/cpu-target.c b/cpu-target.c
index 667688332c9..11592e2583f 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -172,12 +172,9 @@ void cpu_exec_unrealizefn(CPUState *cpu)
     }
 #endif
 
-    cpu_list_remove(cpu);
-    /*
-     * Now that the vCPU has been removed from the RCU list, we can call
-     * accel_cpu_common_unrealize, which may free fields using call_rcu.
-     */
     accel_cpu_common_unrealize(cpu);
+
+    cpu_list_remove(cpu);
 }
 
 /*
-- 
2.47.1

Re: [RFC PATCH 3/9] cpus: Remove cpu from global queue after UNREALIZE completed

Posted by Richard Henderson 2 months ago

On 1/28/25 06:21, Philippe Mathieu-Daudé wrote:
> Previous commit removed the restriction on completing the full QDev
> UNREALIZE step before removing vCPUs from global queue, it is now
> safe to call cpu_list_remove() after accel_cpu_common_unrealize().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   cpu-target.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/cpu-target.c b/cpu-target.c
> index 667688332c9..11592e2583f 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -172,12 +172,9 @@ void cpu_exec_unrealizefn(CPUState *cpu)
>       }
>   #endif
>   
> -    cpu_list_remove(cpu);
> -    /*
> -     * Now that the vCPU has been removed from the RCU list, we can call
> -     * accel_cpu_common_unrealize, which may free fields using call_rcu.
> -     */
>       accel_cpu_common_unrealize(cpu);
> +
> +    cpu_list_remove(cpu);
>   }

I don't believe this is correct.  Why would we have an unrealized cpu on the list?  What's 
wrong with removing the cpu from the list before unrealize?


r~