From: Yodel Eldar via <qemu-devel@nongnu.org>
To: "Alex Bennée" <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Cc: Alexandre Iooss <erdnaxe@crans.org>,
Mahmoud Mandour <ma.mandourr@gmail.com>,
Pierrick Bouvier <pierrick.bouvier@linaro.org>
Subject: Re: [RFC PATCH] plugins: shorten aggressively long name
Date: Fri, 28 Nov 2025 11:16:24 -0600 [thread overview]
Message-ID: <1a92fec0-4dcb-43e7-9748-a0a9fbb01cfb@yodel.dev> (raw)
In-Reply-To: <7b2af00b-b188-43ab-bb0c-f041ab45ebb0@yodel.dev>
On 28/11/2025 10:40, Yodel Eldar via wrote:
> Hi, Alex!
>
> On 21/11/2025 07:03, Alex Bennée wrote:
>> The old name comes in at a 51 characters, contains at least one
>> redundant token and exec is arguably implied by inline as all inline
>> operations occur when instructions are executing.
>>
>> By putting the name on a substantial diet we can reduce it by 15% and
>> gain valuable white-space in the process.
>
> Thanks for proposing this: 51 characters does seem rather excessive!
>
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> include/qemu/qemu-plugin.h | 4 ++--
>> contrib/plugins/cflow.c | 22 +++++++++++-----------
>> contrib/plugins/howvec.c | 2 +-
>> contrib/plugins/stoptrigger.c | 10 ++++++----
>> plugins/api.c | 2 +-
>> tests/tcg/plugins/discons.c | 18 +++++++++---------
>> tests/tcg/plugins/inline.c | 6 +++---
>> tests/tcg/plugins/insn.c | 5 +++--
>> 8 files changed, 36 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index 60de4fdd3fa..29663591ebf 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -516,7 +516,7 @@ void qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> void *userdata);
>> /**
>> - * qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu() - insn exec
>> inline op
>> + * qemu_plugin_register_inline_per_vcpu() - insn exec inline op
>> * @insn: the opaque qemu_plugin_insn handle for an instruction
>> * @op: the type of qemu_plugin_op (e.g. ADD_U64)
>> * @entry: entry to run op
>> @@ -525,7 +525,7 @@ void qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> * Insert an inline op to every time an instruction executes.
>> */
>> QEMU_PLUGIN_API
>> -void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> +void qemu_plugin_register_inline_per_vcpu(
>> struct qemu_plugin_insn *insn,
>> enum qemu_plugin_op op,
>> qemu_plugin_u64 entry,
>
> Could we preserve naming consistency with its sibling functions, tb and
> mem, by, say, removing the "_per_vcpu" thereby implying thread-safety
> instead? I.e.,
>
> qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu ->
> qemu_plugin_register_vcpu_insn[_exec]_inline
>
> qemu_plugin_register_vcpu_mem_inline_per_vcpu ->
> qemu_plugin_register_vcpu_mem_inline
>
> qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu ->
> qemu_plugin_register_vcpu_tb[_exec]_inline
>
> Since the series, 20240304130036.124418-1-pierrick.bouvier@linaro.org,
> that introduced the cycle also removed the non-thread-safe versions in
> fba3b490a, perhaps we could recycle the retired names sans the arguably
> redundant "exec."
>
> If we do remove the "exec" tokens, we should probably do the same for
> the corresponding callback functions, too:
>
> qemu_plugin_register_vcpu_insn_exec_cb ->
> qemu_plugin_register_vcpu_insn_cb ->
>
> qemu_plugin_register_vcpu_tb_exec_cb ->
> qemu_plugin_register_vcpu_tb_cb
>
Nevermind about the callbacks: these don't enjoy implication via
"inline." Sorry about the noise.
> Thanks,
> Yodel
>
>> diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
>> index b5e33f25f9b..cef5ae2239f 100644
>> --- a/contrib/plugins/cflow.c
>> +++ b/contrib/plugins/cflow.c
>> @@ -320,14 +320,14 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> * check where we are at. Do this on the first instruction and not
>> * the TB so we don't get mixed up with above.
>> */
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(first_insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - end_block,
>> qemu_plugin_insn_vaddr(last_insn));
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(first_insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - pc_after_block,
>> -
>> qemu_plugin_insn_vaddr(last_insn) +
>> -
>> qemu_plugin_insn_size(last_insn));
>> + qemu_plugin_register_inline_per_vcpu(first_insn,
>> + QEMU_PLUGIN_INLINE_STORE_U64,
>> + end_block,
>> qemu_plugin_insn_vaddr(last_insn));
>> + qemu_plugin_register_inline_per_vcpu(first_insn,
>> + QEMU_PLUGIN_INLINE_STORE_U64,
>> + pc_after_block,
>> +
>> qemu_plugin_insn_vaddr(last_insn) +
>> +
>> qemu_plugin_insn_size(last_insn));
>> for (int idx = 0; idx < qemu_plugin_tb_n_insns(tb); ++idx) {
>> struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb,
>> idx);
>> @@ -355,9 +355,9 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> }
>> /* Store the PC of what we are about to execute */
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - last_pc,
>> ipc);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> + last_pc, ipc);
>> }
>> }
>> diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
>> index 42bddb6566d..c60737d57f1 100644
>> --- a/contrib/plugins/howvec.c
>> +++ b/contrib/plugins/howvec.c
>> @@ -321,7 +321,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> if (cnt) {
>> if (do_inline) {
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> + qemu_plugin_register_inline_per_vcpu(
>> insn, QEMU_PLUGIN_INLINE_ADD_U64,
>> qemu_plugin_scoreboard_u64(cnt), 1);
>> } else {
>> diff --git a/contrib/plugins/stoptrigger.c b/contrib/plugins/
>> stoptrigger.c
>> index b3a6ed66a7b..68c0ed432af 100644
>> --- a/contrib/plugins/stoptrigger.c
>> +++ b/contrib/plugins/stoptrigger.c
>> @@ -73,10 +73,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> if (exit_on_icount) {
>> /* Increment and check scoreboard for each instruction */
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> - insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_count, 1);
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> - insn, QEMU_PLUGIN_INLINE_STORE_U64, current_pc,
>> insn_vaddr);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_ADD_U64,
>> + insn_count, 1);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> + current_pc,
>> insn_vaddr);
>> qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> insn, exit_icount_reached, QEMU_PLUGIN_CB_NO_REGS,
>> QEMU_PLUGIN_COND_EQ, insn_count, icount + 1, NULL);
>> diff --git a/plugins/api.c b/plugins/api.c
>> index eac04cc1f6b..267fa2fd503 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -154,7 +154,7 @@ void qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> cond, entry, imm, udata);
>> }
>> -void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> +void qemu_plugin_register_inline_per_vcpu(
>> struct qemu_plugin_insn *insn,
>> enum qemu_plugin_op op,
>> qemu_plugin_u64 entry,
>> diff --git a/tests/tcg/plugins/discons.c b/tests/tcg/plugins/discons.c
>> index 2e0e664e823..1348d6e5020 100644
>> --- a/tests/tcg/plugins/discons.c
>> +++ b/tests/tcg/plugins/discons.c
>> @@ -156,15 +156,15 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> uint64_t next_pc = pc + qemu_plugin_insn_size(insn);
>> uint64_t has_next = (i + 1) < n_insns;
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - last_pc,
>> pc);
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - from_pc,
>> next_pc);
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
>> -
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> - has_from,
>> has_next);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> + last_pc, pc);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> + from_pc, next_pc);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_STORE_U64,
>> + has_from, has_next);
>> qemu_plugin_register_vcpu_insn_exec_cb(insn, insn_exec,
>>
>> QEMU_PLUGIN_CB_NO_REGS, NULL);
>> }
>> diff --git a/tests/tcg/plugins/inline.c b/tests/tcg/plugins/inline.c
>> index 73dde995781..35307501105 100644
>> --- a/tests/tcg/plugins/inline.c
>> +++ b/tests/tcg/plugins/inline.c
>> @@ -244,15 +244,15 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> void *insn_store = insn;
>> void *mem_store = (char *)insn_store + 0xff;
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> + qemu_plugin_register_inline_per_vcpu(
>> insn, QEMU_PLUGIN_INLINE_STORE_U64, data_insn,
>> (uintptr_t) insn_store);
>> qemu_plugin_register_vcpu_insn_exec_cb(
>> insn, vcpu_insn_exec, QEMU_PLUGIN_CB_NO_REGS, insn_store);
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> + qemu_plugin_register_inline_per_vcpu(
>> insn, QEMU_PLUGIN_INLINE_ADD_U64, count_insn_inline, 1);
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> + qemu_plugin_register_inline_per_vcpu(
>> insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_cond_track_count,
>> 1);
>> qemu_plugin_register_vcpu_insn_exec_cond_cb(
>> insn, vcpu_insn_cond_exec, QEMU_PLUGIN_CB_NO_REGS,
>> diff --git a/tests/tcg/plugins/insn.c b/tests/tcg/plugins/insn.c
>> index 0c723cb9ed8..b337fda9f13 100644
>> --- a/tests/tcg/plugins/insn.c
>> +++ b/tests/tcg/plugins/insn.c
>> @@ -147,8 +147,9 @@ static void vcpu_tb_trans(qemu_plugin_id_t id,
>> struct qemu_plugin_tb *tb)
>> struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>> if (do_inline) {
>> - qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
>> - insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_count, 1);
>> + qemu_plugin_register_inline_per_vcpu(insn,
>> +
>> QEMU_PLUGIN_INLINE_ADD_U64,
>> + insn_count, 1);
>> } else {
>> qemu_plugin_register_vcpu_insn_exec_cb(
>> insn, vcpu_insn_exec_before, QEMU_PLUGIN_CB_NO_REGS,
>> NULL);
>
>
next prev parent reply other threads:[~2025-11-28 17:16 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-21 13:03 [RFC PATCH] plugins: shorten aggressively long name Alex Bennée
2025-11-28 16:40 ` Yodel Eldar via
2025-11-28 17:16 ` Yodel Eldar via [this message]
2025-12-01 15:29 ` Pierrick Bouvier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1a92fec0-4dcb-43e7-9748-a0a9fbb01cfb@yodel.dev \
--to=qemu-devel@nongnu.org \
--cc=alex.bennee@linaro.org \
--cc=erdnaxe@crans.org \
--cc=ma.mandourr@gmail.com \
--cc=pierrick.bouvier@linaro.org \
--cc=yodel.eldar@yodel.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).