Although unreachable, still unregister the RCU before exiting
the thread, as documented in "qemu/rcu.h":

 /*
  * Important !
  *
  * Each thread containing read-side critical sections must be registered
  * with rcu_register_thread() before calling rcu_read_lock().
  * rcu_unregister_thread() should be called before the thread exits.
  */

Unregister the RCU to be on par with what is done for other
accelerators.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Message-Id: <20250702185332.43650-66-philmd@linaro.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 6eec5c9eee9..a578698d071 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -302,6 +302,8 @@ static void *rr_cpu_thread_fn(void *arg)
         rr_deal_with_unplugged_cpus();
     }
 
+    rcu_unregister_thread();
+
     g_assert_not_reached();
 }
 
-- 
2.49.0

On Fri, 4 Jul 2025 at 14:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Although unreachable, still unregister the RCU before exiting
> the thread, as documented in "qemu/rcu.h":
>
>  /*
>   * Important !
>   *
>   * Each thread containing read-side critical sections must be registered
>   * with rcu_register_thread() before calling rcu_read_lock().
>   * rcu_unregister_thread() should be called before the thread exits.
>   */
>
> Unregister the RCU to be on par with what is done for other
> accelerators.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> Message-Id: <20250702185332.43650-66-philmd@linaro.org>
> ---
>  accel/tcg/tcg-accel-ops-rr.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index 6eec5c9eee9..a578698d071 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -302,6 +302,8 @@ static void *rr_cpu_thread_fn(void *arg)
>          rr_deal_with_unplugged_cpus();
>      }
>
> +    rcu_unregister_thread();
> +
>      g_assert_not_reached();
>  }

This has reintroduced CID 1547782 (unreachable code).

We can't get to this point, so why are we trying to call a
function here ? This is not a place where the thread can exit.

thanks
-- PMM

On 10/7/25 16:33, Peter Maydell wrote:
> On Fri, 4 Jul 2025 at 14:25, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Although unreachable, still unregister the RCU before exiting
>> the thread, as documented in "qemu/rcu.h":
>>
>>   /*
>>    * Important !
>>    *
>>    * Each thread containing read-side critical sections must be registered
>>    * with rcu_register_thread() before calling rcu_read_lock().
>>    * rcu_unregister_thread() should be called before the thread exits.
>>    */
>>
>> Unregister the RCU to be on par with what is done for other
>> accelerators.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>> Message-Id: <20250702185332.43650-66-philmd@linaro.org>
>> ---
>>   accel/tcg/tcg-accel-ops-rr.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
>> index 6eec5c9eee9..a578698d071 100644
>> --- a/accel/tcg/tcg-accel-ops-rr.c
>> +++ b/accel/tcg/tcg-accel-ops-rr.c
>> @@ -302,6 +302,8 @@ static void *rr_cpu_thread_fn(void *arg)
>>           rr_deal_with_unplugged_cpus();
>>       }
>>
>> +    rcu_unregister_thread();
>> +
>>       g_assert_not_reached();
>>   }
> 
> This has reintroduced CID 1547782 (unreachable code).
> 
> We can't get to this point, so why are we trying to call a
> function here ? This is not a place where the thread can exit.

The goal is to unify accelerators vcpu thread logic and
eventually remove AccelOpsClass::create_vcpu_thread(), superseded
by AccelOpsClass::cpu_thread_routine(). I couldn't finish RR for
10.1 but squeezed this patch in. Let's revert it, and I'll
re-commit it later.

On Fri, 11 Jul 2025 at 11:45, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 10/7/25 16:33, Peter Maydell wrote:
> > This has reintroduced CID 1547782 (unreachable code).
> >
> > We can't get to this point, so why are we trying to call a
> > function here ? This is not a place where the thread can exit.
>
> The goal is to unify accelerators vcpu thread logic and
> eventually remove AccelOpsClass::create_vcpu_thread(), superseded
> by AccelOpsClass::cpu_thread_routine(). I couldn't finish RR for
> 10.1 but squeezed this patch in. Let's revert it, and I'll
> re-commit it later.

When you do have a need for it, note that rcu_unregister_thread()
is probably not the only call you need here -- see commit
da7510b720591. We used to also have an equally unreachable
rcu_remove_force_rcu_notifier() call.

thanks
-- PMM