On 6/16/2026 9:12 AM, Harry van Haaren | OPENCHIP wrote: > From: Harry van Haaren <[email protected]> >
No needed for this From: tag since you are already the sender and have a signed-off-by. > The existing code in execlog was never upgraded to the Scoreboard > API, resulting in a bespoke implementation of per-vCPU datastructure > handling. This had some race-conditions, and causes segfaults with > a simple multi-threaded program and two instances of execlog running. > > The patch here refactors the custom GArray and GRWLock code away, and > uses the scoreboard APIs like the other plugins. This solves the > "printing while expanding" race-condition of two plugins with multiple > threads in the guest, and hence fixes a segfault. > Great! > Signed-off-by: Harry van Haaren <[email protected]> > > --- > > Hi Qemu-Devel! > > This is my first contribution to QEMU itself, but I have worked on > DPDK, OVS, and QEMU-RS a bit in the past. I have experience with > mailing lists for patches and some QEMU internals. I've read the > 'submitting a patch' guide, and ran checkpatch... so basics should > be OK I hope! > > Please note that LLMs were used to investigate and fix this bug, > but all code in this patch has been reviewed by me, and I believe > it to be a good solution. > --- > contrib/plugins/execlog.c | 65 +++++++++++++++------------------------ > 1 file changed, 25 insertions(+), 40 deletions(-) > > diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c > index d00d9c4ff3..e51af9f2df 100644 > --- a/contrib/plugins/execlog.c > +++ b/contrib/plugins/execlog.c > @@ -31,33 +31,28 @@ typedef struct CPU { > > QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > > -static GArray *cpus; > -static GRWLock expand_array_lock; > +/* > + * Per-vCPU state stored in a qemu_plugin_scoreboard. The scoreboard manages > + * per-vCPU storage automatically, eliminating the need for manual array > + * growth, locks, or pointer-stability workarounds. > + */ > +static struct qemu_plugin_scoreboard *cpus; > > static GPtrArray *imatches; > static GArray *amatches; > static GPtrArray *rmatches; > static bool disas_assist; > static GMutex add_reg_name_lock; > +static GMutex execlog_output_mutex; QEMU log already has some locking mechanism, you just need to make sure you output everything in a single call at a time if you want atomicity. To help with string manipulation, you can use a GString to build a message. (see other plugins, it's a common pattern). So you can remove this mutex and code associated, and make sure you have a single call to qemu_plugin_outs in every function. > static GPtrArray *all_reg_names; > > -static CPU *get_cpu(int vcpu_index) > -{ > - CPU *c; > - g_rw_lock_reader_lock(&expand_array_lock); > - c = &g_array_index(cpus, CPU, vcpu_index); > - g_rw_lock_reader_unlock(&expand_array_lock); > - > - return c; > -} > - > /** > * Add memory read or write information to current instruction log > */ > static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info, > uint64_t vaddr, void *udata) > { > - CPU *c = get_cpu(cpu_index); > + CPU *c = qemu_plugin_scoreboard_find(cpus, cpu_index); > GString *s = c->last_exec; > > /* Find vCPU in array */ > @@ -117,7 +112,7 @@ static void insn_check_regs(CPU *cpu) > /* Log last instruction while checking registers */ > static void vcpu_insn_exec_with_regs(unsigned int cpu_index, void *udata) > { > - CPU *cpu = get_cpu(cpu_index); > + CPU *cpu = qemu_plugin_scoreboard_find(cpus, cpu_index); > > /* Print previous instruction in cache */ > if (cpu->last_exec->len) { > @@ -125,8 +120,10 @@ static void vcpu_insn_exec_with_regs(unsigned int > cpu_index, void *udata) > insn_check_regs(cpu); > } > > + g_mutex_lock(&execlog_output_mutex); > qemu_plugin_outs(cpu->last_exec->str); > qemu_plugin_outs("\n"); > + g_mutex_unlock(&execlog_output_mutex); > } > > /* Store new instruction in cache */ > @@ -138,7 +135,7 @@ static void vcpu_insn_exec_with_regs(unsigned int > cpu_index, void *udata) > /* Log last instruction while checking registers, ignore next */ > static void vcpu_insn_exec_only_regs(unsigned int cpu_index, void *udata) > { > - CPU *cpu = get_cpu(cpu_index); > + CPU *cpu = qemu_plugin_scoreboard_find(cpus, cpu_index); > > /* Print previous instruction in cache */ > if (cpu->last_exec->len) { > @@ -146,8 +143,10 @@ static void vcpu_insn_exec_only_regs(unsigned int > cpu_index, void *udata) > insn_check_regs(cpu); > } > > + g_mutex_lock(&execlog_output_mutex); > qemu_plugin_outs(cpu->last_exec->str); > qemu_plugin_outs("\n"); > + g_mutex_unlock(&execlog_output_mutex); > } > > /* reset */ > @@ -157,12 +156,14 @@ static void vcpu_insn_exec_only_regs(unsigned int > cpu_index, void *udata) > /* Log last instruction without checking regs, setup next */ > static void vcpu_insn_exec(unsigned int cpu_index, void *udata) > { > - CPU *cpu = get_cpu(cpu_index); > + CPU *cpu = qemu_plugin_scoreboard_find(cpus, cpu_index); > > /* Print previous instruction in cache */ > if (cpu->last_exec->len) { > + g_mutex_lock(&execlog_output_mutex); > qemu_plugin_outs(cpu->last_exec->str); > qemu_plugin_outs("\n"); > + g_mutex_unlock(&execlog_output_mutex); > } > > /* Store new instruction in cache */ > @@ -378,21 +379,10 @@ static GPtrArray *registers_init(int vcpu_index) > * - last_exec tracking data > * - list of tracked registers > * - initial value of registers > - * > - * As we could have multiple threads trying to do this we need to > - * serialise the expansion under a lock. > */ > static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index) > { > - CPU *c; > - > - g_rw_lock_writer_lock(&expand_array_lock); > - if (vcpu_index >= cpus->len) { > - g_array_set_size(cpus, vcpu_index + 1); > - } > - g_rw_lock_writer_unlock(&expand_array_lock); > - > - c = get_cpu(vcpu_index); > + CPU *c = qemu_plugin_scoreboard_find(cpus, vcpu_index); > c->last_exec = g_string_new(NULL); > c->registers = registers_init(vcpu_index); > } > @@ -402,16 +392,15 @@ static void vcpu_init(qemu_plugin_id_t id, unsigned int > vcpu_index) > */ > static void plugin_exit(qemu_plugin_id_t id, void *p) > { > - guint i; > - g_rw_lock_reader_lock(&expand_array_lock); > - for (i = 0; i < cpus->len; i++) { > - CPU *c = get_cpu(i); > - if (c->last_exec && c->last_exec->str) { > + int n = qemu_plugin_num_vcpus(); > + for (int i = 0; i < n; i++) { > + CPU *c = qemu_plugin_scoreboard_find(cpus, i); > + if (c->last_exec && c->last_exec->len) { > qemu_plugin_outs(c->last_exec->str); > qemu_plugin_outs("\n"); > } > } > - g_rw_lock_reader_unlock(&expand_array_lock); > + qemu_plugin_scoreboard_free(cpus); > } > > /* Add a match to the array of matches */ > @@ -452,12 +441,8 @@ QEMU_PLUGIN_EXPORT int > qemu_plugin_install(qemu_plugin_id_t id, > const qemu_info_t *info, int argc, > char **argv) > { > - /* > - * Initialize dynamic array to cache vCPU instruction. In user mode > - * we don't know the size before emulation. > - */ > - cpus = g_array_sized_new(true, true, sizeof(CPU), > - info->system_emulation ? info->system.max_vcpus > : 1); > + /* Initialize scoreboard to cache per-vCPU instruction state. */ > + cpus = qemu_plugin_scoreboard_new(sizeof(CPU)); > > for (int i = 0; i < argc; i++) { > char *opt = argv[i]; > -- > 2.54.0 > > > > The content, data, and any attached documents to this email are addressed > exclusively to the addressee and are confidential and/or may be subject to a > non-disclosure agreement. Any use, forwarding, disclosure, and/or copying, in > whole or in part, without authorization is prohibited. If you have received > this email in error, we apologize and, please notify the sender or Openchip > immediately, and delete it from your system. > > El contenido, los datos y cualquier documento adjunto a este correo > electrónico están dirigidos exclusivamente al destinatario y son > confidenciales y/o pueden estar sujetas a un acuerdo de no revelación. Está > prohibido cualquier uso, reenvío, divulgación o copia, total o parcial, sin > autorización. Si has recibido este correo por error, te pedimos disculpas y > agradecemos que lo notifiques de inmediato al remitente o a Openchip, y lo > elimines de tu sistema. > > El contingut, les dades i qualsevol document adjunt a aquest correu > electrònic estan dirigits exclusivament al destinatari i són confidencials > i/o poden estar subjectes a un acord de no revelació. Està prohibit qualsevol > ús, reenviament, divulgació o còpia, total o parcial, sense autorització. Si > has rebut aquest correu per error, et demanem disculpes i agraïm que ho > notifiquis d'immediat al remitent o a Openchip, i l'eliminis del teu sistema Regards, Pierrick
