Re: [PATCH v2 08/12] plugins: Use tb_flush__exclusive

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 23/9/25 22:28, Richard Henderson wrote:
> On 9/23/25 06:35, Philippe Mathieu-Daudé wrote:
>> On 23/9/25 04:39, Richard Henderson wrote:
>>> In all cases, we are already within start_exclusive.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> Cc: Alex Bennée" <alex.bennee@linaro.org>
>>> Cc: Alexandre Iooss <erdnaxe@crans.org>
>>> Cc: Mahmoud Mandour <ma.mandourr@gmail.com>
>>> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>   plugins/core.c   | 6 ++----
>>>   plugins/loader.c | 2 +-
>>>   2 files changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/plugins/core.c b/plugins/core.c
>>> index c6e9ef1478..4ae1a6ae17 100644
>>> --- a/plugins/core.c
>>> +++ b/plugins/core.c
>>> @@ -248,7 +248,7 @@ static void 
>>> plugin_grow_scoreboards__locked(CPUState *cpu)
>>>           }
>>>           plugin.scoreboard_alloc_size = scoreboard_size;
>>>           /* force all tb to be flushed, as scoreboard pointers were 
>>> changed. */
>>> -        tb_flush(cpu);
>>> +        tb_flush__exclusive();
>>>       }
>>>       end_exclusive();
>>>   }
>>> @@ -684,8 +684,6 @@ void qemu_plugin_user_exit(void)
>>>        * with the one in fork_start(). That is:
>>>        * - start_exclusive(), which acquires qemu_cpu_list_lock,
>>>        *   must be called before acquiring plugin.lock.
>>> -     * - tb_flush(), which acquires mmap_lock(), must be called
>>> -     *   while plugin.lock is not held.
>>>        */
>>>       start_exclusive();
>>> @@ -705,7 +703,7 @@ void qemu_plugin_user_exit(void)
>>>       }
>>>       qemu_rec_mutex_unlock(&plugin.lock);
>>> -    tb_flush(current_cpu);
>>> +    tb_flush__exclusive();
>>>       end_exclusive();
>>>       /* now it's safe to handle the exit case */
>>
>> Hmm it seems we are triggering again the issue reported about
>> TARGET_NR_exit_group in https://linaro.atlassian.net/browse/QEMU-706:
>>
>>    "Under user emulation, threads can exit via pthread_join or at
>>     the end of the process via exit_group syscall.
>>
>>    The current plugin exit hook affects all vcpus (see
>>    qemu_plugin_disable_mem_helpers call in qemu_plugin_user_exit)."
>>
>> Crash log:
>>
>> qemu-loongarch64: ../../accel/tcg/tb-maint.c:94: tb_remove_all: 
>> Assertion `have_mmap_lock()' failed.
>>
>> Thread 1 "qemu-loongarch6" received signal SIGABRT, Aborted.
>> __pthread_kill_implementation (no_tid=0, signo=6, 
>> threadid=140737340860416) at ./nptl/ pthread_kill.c:44
>> 44    ./nptl/pthread_kill.c: No such file or directory.
>> (gdb) bt
>> #0  __pthread_kill_implementation (no_tid=0, signo=6, 
>> threadid=140737340860416) at ./nptl/ pthread_kill.c:44
>> #1  __pthread_kill_internal (signo=6, threadid=140737340860416) at ./ 
>> nptl/pthread_kill.c:78
>> #2  __GI___pthread_kill (threadid=140737340860416, 
>> signo=signo@entry=6) at ./nptl/ pthread_kill.c:89
>> #3  0x00007ffff746f476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/ 
>> posix/raise.c:26
>> #4  0x00007ffff74557f3 in __GI_abort () at ./stdlib/abort.c:79
>> #5  0x00007ffff745571b in __assert_fail_base (fmt=0x7ffff760a130 
>> "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x555555733f0c 
>> "have_mmap_lock()",
>>      file=0x555555733ef1 "../../accel/tcg/tb-maint.c", line=94, 
>> function=<optimized out>) at ./assert/assert.c:94
>> #6  0x00007ffff7466e96 in __GI___assert_fail 
>> (assertion=assertion@entry=0x555555733f0c "have_mmap_lock()",
>>      file=file@entry=0x555555733ef1 "../../accel/tcg/tb-maint.c", 
>> line=line@entry=94, function=function@entry=0x555555734038 
>> <__PRETTY_FUNCTION__.8> "tb_remove_all")
>>      at ./assert/assert.c:103
>> #7  0x0000555555612e41 in tb_remove_all () at ../../accel/tcg/tb- 
>> maint.c:94
>> #8  tb_flush__exclusive () at ../../accel/tcg/tb-maint.c:781
>> #9  0x0000555555623a0c in qemu_plugin_user_exit () at ../../plugins/ 
>> core.c:706
>> #10 0x0000555555696e54 in preexit_cleanup (env=<optimized out>, 
>> code=code@entry=0) at ../../linux-user/exit.c:36
> 
> I fixed this by replacing the assert in the user-only version of 
> tb_remove_all.

Clever shortcut :)