On 7/15/2026 10:05 AM, Harry van Haaren wrote:
>> From: Pierrick Bouvier <[email protected]>
>> Sent: Tuesday 16 June 2026 7:40 pm
>> To: Harry van Haaren | OPENCHIP <[email protected]>; 
>> [email protected] <[email protected]>
>> Cc: [email protected] <[email protected]>; [email protected] 
>> <[email protected]>
>> Subject: Re: [PATCH 2/2] plugins/execlog: fix execlog vcpu_exit execution 
>> print loss
>>  
>> On 6/16/2026 9:13 AM, Harry van Haaren | OPENCHIP wrote:
>>> From: Harry van Haaren <[email protected]>
>>>
>>> Executed instructions are cached in string format inside the
>>> execlog plugin. These strings are flushed on exit of a TB, improving
>>> performance. This causes executed instructions to be lost when an
>>> 'ecall' (riscv system call) occurs that causes the thread to terminate.
>>>
>>> The fix in this patch registers an 'on_exit()' callback, and flushes
>>> any content in the c->last_exec buffer, to ensure all instructions are
>>> present in the final instruction log.
>>>
>>> A mutex lock/unlock is added around the plugin exit, to ensure the
>>> lines are not corrupted.
>>>
>>> Signed-off-by: Harry van Haaren <[email protected]>
> 
> <snip>
> 
>>> +/**
>>> + * On plugin exit, flush any remaining cached instructions and free state.
>>>    */
>>>   static void plugin_exit(qemu_plugin_id_t id, void *p)
>>>   {
>>> @@ -396,8 +417,10 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>>>       for (int i = 0; i < n; i++) {
>>>           CPU *c = qemu_plugin_scoreboard_find(cpus, i);
>>>           if (c->last_exec && c->last_exec->len) {
>>> +            g_mutex_lock(&execlog_output_mutex);
>>>               qemu_plugin_outs(c->last_exec->str);
>>>               qemu_plugin_outs("\n");
>>> +            g_mutex_unlock(&execlog_output_mutex);
>>>           }
>>>       }
>>>       qemu_plugin_scoreboard_free(cpus);
>>> @@ -467,6 +490,7 @@ QEMU_PLUGIN_EXPORT int 
>>> qemu_plugin_install(qemu_plugin_id_t id,
>>>
>>>       /* Register init, translation block and exit callbacks */
>>>       qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
>>> +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
>>>       qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>>       qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>>>
>>
>> Why not simply make this flush in atexit callback?
> 
> I think on guest-thread join, the pthread stops, and qemu calls vcpu exit 
> callback.
> At that point in time, its nice to flush the "ecall" (to printf or disk or 
> whatever the output is) so the user sees it immidiately.
> 
> The risk of not flushing here, is we wait until:
> A) the plugin (not thread) exits, which could cause other threads to 
> interleave/finish first (confusing, interleaved and delayed output)

Taking a look here, I can see that we have two different mechanic for
atexit: one for system (relies on libc atexit), and one for user, which
indeed can race between threads.

It should be refactored, I'll take a look to remove the need to use any
lock in atexit handlers for plugins.

> or
> B) a new thread is spawned by the guest, and the vcpu is re-used. This will 
> correctly flush the previous-thread final instruction, but after an 
> application-dependent delay.
>

That's a good concern, and fixing it with vcpu_exit seems like a good
idea given this argument.

> Given that thread-exit is already heavy-weight and rare, I think the best for 
> the user that the final instruction flushes immediately when a thread is 
> finished.
> Unless I get strong feedback for a different approach, I'll respin the 
> patchset and send (with git-publish, with footer removed, and "| Openchip" 
> fixed... hopefully!)
> 
> Regards, -Harry

Yes, you can keep the current approach.

Thanks,
Pierrick

Reply via email to