From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
To: Gustavo Romero <gustavo.romero@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH v5 0/9] TCG plugins new inline operations
Date: Thu, 2 May 2024 14:16:38 -0700 [thread overview]
Message-ID: <38188b6e-fa36-4c14-9f0f-763826bc9363@linaro.org> (raw)
In-Reply-To: <6f37fbce-9fcc-ab7d-08fc-cfd8bb470f9b@linaro.org>
On 5/2/24 13:48, Gustavo Romero wrote:
> Hi Pierrick,
>
> On 5/2/24 5:09 PM, Gustavo Romero wrote:
>> On 5/2/24 4:45 PM, Pierrick Bouvier wrote:
>>> Hi Gustavo,
>>>
>>> On 5/2/24 12:16, Gustavo Romero wrote:
>>>> Hi Pierrick,
>>>>
>>>> On 5/2/24 3:08 PM, Pierrick Bouvier wrote:
>>>>> This series implement two new operations for plugins:
>>>>> - Store inline allows to write a specific value to a scoreboard.
>>>>> - Conditional callback executes a callback only when a given condition is true.
>>>>> The condition is evaluated inline.
>>>>>
>>>>> It's possible to mix various inline operations (add, store) with conditional
>>>>> callbacks, allowing efficient "trap" based counters.
>>>>>
>>>>> It builds on top of new scoreboard API, introduced in the previous series.
>>>>>
>>>>> NOTE: Two patches still need review
>>>>>
>>>>> v2
>>>>> --
>>>>>
>>>>> - fixed issue with udata not being passed to conditional callback
>>>>> - added specific test for this in tests/plugin/inline.c (udata was NULL before).
>>>>>
>>>>> v3
>>>>> --
>>>>>
>>>>> - rebased on top of "plugins: Rewrite plugin code generation":
>>>>> 20240316015720.3661236-1-richard.henderson@linaro.org
>>>>> - single pass code generation
>>>>> - small cleanups for code generation
>>>>>
>>>>> v4
>>>>> --
>>>>>
>>>>> - remove op field from qemu_plugin_inline_cb
>>>>> - use tcg_constant_i64 to load immediate value to store
>>>>>
>>>>> v5
>>>>> --
>>>>>
>>>>> - rebase on top of master now that Richard's series was merged
>>>>>
>>>>> Pierrick Bouvier (9):
>>>>> plugins: prepare introduction of new inline ops
>>>>> plugins: extract generate ptr for qemu_plugin_u64
>>>>> plugins: add new inline op STORE_U64
>>>>> tests/plugin/inline: add test for STORE_U64 inline op
>>>>> plugins: conditional callbacks
>>>>> tests/plugin/inline: add test for conditional callback
>>>>> plugins: distinct types for callbacks
>>>>> plugins: extract cpu_index generate
>>>>> plugins: remove op from qemu_plugin_inline_cb
>>>>>
>>>>> include/qemu/plugin.h | 42 +++++++----
>>>>> include/qemu/qemu-plugin.h | 80 ++++++++++++++++++++-
>>>>> plugins/plugin.h | 12 +++-
>>>>> accel/tcg/plugin-gen.c | 136 +++++++++++++++++++++++++++--------
>>>>> plugins/api.c | 39 ++++++++++
>>>>> plugins/core.c | 109 ++++++++++++++++++++--------
>>>>> tests/plugin/inline.c | 130 +++++++++++++++++++++++++++++++--
>>>>> plugins/qemu-plugins.symbols | 2 +
>>>>> 8 files changed, 466 insertions(+), 84 deletions(-)
>>>>
>>>> The description in the commit message of patches 1/9, 2/9, 6/9, 7/9, and 8/9 is missing.
>>>>
>>>> Is this intentional?
>>>>
>>>
>>> Do you mean there is no multiline commit message for those changes?
>>> Indeed, for some of those patches, the change is a single line commit message.
>>
>> I just see a commit title and the Signed-off-by. For example, in 8/9
>> I see the following on git log:
>>
>> commit f518898aa09b42e317b887237bb75a432b477c6d
>> Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Date: Thu May 2 11:08:46 2024 -0700
>>
>> plugins: extract cpu_index generate
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> It has only the title: "plugins: extract cpu_index generate"
>> and R-b and S-b, so no description about the changes.
>
> The description (the body of the commit message) is mentioned here:
>
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message
>
> After quickly looking at the committed logs, I do notice some commits missing it,
> but I really believe it's important to have it for the reasons outlined in these
> guidelines.
>
You're right, I just sent a v6 with more elaborated commit messages.
Thanks,
Pierrick
>
> Cheers,
> Gustavo
prev parent reply other threads:[~2024-05-02 21:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-02 18:08 [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 1/9] plugins: prepare introduction of new inline ops Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 2/9] plugins: extract generate ptr for qemu_plugin_u64 Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 3/9] plugins: add new inline op STORE_U64 Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 4/9] tests/plugin/inline: add test for STORE_U64 inline op Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 5/9] plugins: conditional callbacks Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 6/9] tests/plugin/inline: add test for conditional callback Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 7/9] plugins: distinct types for callbacks Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 8/9] plugins: extract cpu_index generate Pierrick Bouvier
2024-05-02 18:08 ` [PATCH v5 9/9] plugins: remove op from qemu_plugin_inline_cb Pierrick Bouvier
2024-05-02 18:21 ` [PATCH v5 0/9] TCG plugins new inline operations Pierrick Bouvier
2024-05-02 19:16 ` Gustavo Romero
2024-05-02 19:45 ` Pierrick Bouvier
2024-05-02 20:09 ` Gustavo Romero
2024-05-02 20:48 ` Gustavo Romero
2024-05-02 21:16 ` Pierrick Bouvier [this message]
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=38188b6e-fa36-4c14-9f0f-763826bc9363@linaro.org \
--to=pierrick.bouvier@linaro.org \
--cc=gustavo.romero@linaro.org \
--cc=qemu-devel@nongnu.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).