[PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[Alex Bradbury]

This is a resend of my previous patchset which unfortunately seems not to have
been applied (see
<https://lore.kernel.org/qemu-devel/cover.1753857212.git.asb@igalia.com/).
I've rebased on current HEAD, checked it works as expected, and added
Reviewed-by tags to the patches, which all received review.

Repeating the summary from last time:

This series contains one minor feature addition and a series of small
bugfixes/improvements. The addition that motivates the submission is to add a
limit argument for the hotblocks plugin, allowing you to control how many
blocks are printed rather than being hardcoded to the 20 most executed.
Setting limit=0 and dumping information about all executed blocks is
incredibly helpful for an analysis script I have downstream.

This is my first contribution to QEMU. I've attempted to follow all of the
guidance in the "Submitting a Patch" guide, but apologies if I missed
anything.

Alex Bradbury (5):
  contrib/plugins/hotblocks: Correctly free sorted counts list
  contrib/plugins/hotblocks: Fix off by one error in iteration of sorted
    blocks
  contrib/plugins/hotblocks: Print uint64_t with PRIu64 rather than
    PRId64
  docs/about/emulation: Add documentation for hotblocks plugin arguments
  contrib/plugins/hotblocks: Allow limit to be set as a command line
    argument

 contrib/plugins/hotblocks.c | 20 ++++++++++++++------
 docs/about/emulation.rst    | 12 ++++++++++++
 2 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.52.0

[PATCH v2 1/5] contrib/plugins/hotblocks: Correctly free sorted counts list

[Alex Bradbury]

g_list_free should be passed the head of the list.

Signed-off-by: Alex Bradbury <asb@igalia.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/hotblocks.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 98404b6885..d3dd23ed9f 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -73,15 +73,16 @@ static void exec_count_free(gpointer key, gpointer value, gpointer user_data)
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
     g_autoptr(GString) report = g_string_new("collected ");
-    GList *counts, *it;
+    GList *counts, *sorted_counts, *it;
     int i;
 
     g_string_append_printf(report, "%d entries in the hash table\n",
                            g_hash_table_size(hotblocks));
     counts = g_hash_table_get_values(hotblocks);
-    it = g_list_sort_with_data(counts, cmp_exec_count, NULL);
+    sorted_counts = g_list_sort_with_data(counts, cmp_exec_count, NULL);
 
-    if (it) {
+    if (sorted_counts) {
+        it = sorted_counts;
         g_string_append_printf(report, "pc, tcount, icount, ecount\n");
 
         for (i = 0; i < limit && it->next; i++, it = it->next) {
@@ -94,7 +95,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
                     qemu_plugin_scoreboard_u64(rec->exec_count)));
         }
 
-        g_list_free(it);
+        g_list_free(sorted_counts);
     }
 
     qemu_plugin_outs(report->str);
-- 
2.52.0

[PATCH v2 2/5] contrib/plugins/hotblocks: Fix off by one error in iteration of sorted blocks

[Alex Bradbury]

The logic to iterate over the hottest blocks will never reach the last
item in the list, as it checks `it->next != NULL` before entering the
loop. It's hard to trigger this off-by-one error with the default
limit=20, but it is a bug and is problematic if that default is changed
to something larger.

Signed-off-by: Alex Bradbury <asb@igalia.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/hotblocks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index d3dd23ed9f..cf4d6b8c36 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -82,10 +82,9 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     sorted_counts = g_list_sort_with_data(counts, cmp_exec_count, NULL);
 
     if (sorted_counts) {
-        it = sorted_counts;
         g_string_append_printf(report, "pc, tcount, icount, ecount\n");
 
-        for (i = 0; i < limit && it->next; i++, it = it->next) {
+        for (i = 0, it = sorted_counts; i < limit && it; i++, it = it->next) {
             ExecCount *rec = (ExecCount *) it->data;
             g_string_append_printf(
                 report, "0x%016"PRIx64", %d, %ld, %"PRId64"\n",
-- 
2.52.0

[PATCH v2 3/5] contrib/plugins/hotblocks: Print uint64_t with PRIu64 rather than PRId64

[Alex Bradbury]

qemu_plugin_u64_sum returns a uint64_t, so PRIu64 is the correct format
specifier.

Signed-off-by: Alex Bradbury <asb@igalia.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/hotblocks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index cf4d6b8c36..40d8dae1cd 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -87,7 +87,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
         for (i = 0, it = sorted_counts; i < limit && it; i++, it = it->next) {
             ExecCount *rec = (ExecCount *) it->data;
             g_string_append_printf(
-                report, "0x%016"PRIx64", %d, %ld, %"PRId64"\n",
+                report, "0x%016"PRIx64", %d, %ld, %"PRIu64"\n",
                 rec->start_addr, rec->trans_count,
                 rec->insns,
                 qemu_plugin_u64_sum(
-- 
2.52.0

[PATCH v2 4/5] docs/about/emulation: Add documentation for hotblocks plugin arguments

[Alex Bradbury]

Currently just 'inline'.

Signed-off-by: Alex Bradbury <asb@igalia.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 docs/about/emulation.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index 4a7d1f4178..543efc4d7d 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -463,6 +463,16 @@ Example::
   0x000000004002b0, 1, 4, 66087
   ...
 
+Behaviour can be tweaked with the following arguments:
+
+.. list-table:: Hot Blocks plugin arguments
+  :widths: 20 80
+  :header-rows: 1
+
+  * - Option
+    - Description
+  * - inline=true|false
+    - Use faster inline addition of a single counter.
 
 Hot Pages
 .........
-- 
2.52.0

[PATCH v2 5/5] contrib/plugins/hotblocks: Allow limit to be set as a command line argument

[Alex Bradbury]

Also add documentation for this argument. This allows the default of 20
to be overridden, and is helpful for using the hotblocks plugin for
analysis scripts that require collecting data on a larger number of
blocks (e.g. setting limit=0 to dump information on all blocks).

Signed-off-by: Alex Bradbury <asb@igalia.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/hotblocks.c | 10 +++++++++-
 docs/about/emulation.rst    |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 40d8dae1cd..8ecf033997 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -84,7 +84,8 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     if (sorted_counts) {
         g_string_append_printf(report, "pc, tcount, icount, ecount\n");
 
-        for (i = 0, it = sorted_counts; i < limit && it; i++, it = it->next) {
+        for (i = 0, it = sorted_counts; (limit == 0 || i < limit) && it;
+             i++, it = it->next) {
             ExecCount *rec = (ExecCount *) it->data;
             g_string_append_printf(
                 report, "0x%016"PRIx64", %d, %ld, %"PRIu64"\n",
@@ -170,6 +171,13 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
                 fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
                 return -1;
             }
+        } else if (g_strcmp0(tokens[0], "limit") == 0) {
+            char *endptr = NULL;
+            limit = g_ascii_strtoull(tokens[1], &endptr, 10);
+            if (endptr == tokens[1] || *endptr != '\0') {
+                fprintf(stderr, "unsigned integer parsing failed: %s\n", opt);
+                return -1;
+            }
         } else {
             fprintf(stderr, "option parsing failed: %s\n", opt);
             return -1;
diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index 543efc4d7d..e8793b0f9c 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -473,6 +473,8 @@ Behaviour can be tweaked with the following arguments:
     - Description
   * - inline=true|false
     - Use faster inline addition of a single counter.
+  * - limit=N
+    - The number of blocks to be printed. (Default: N = 20, use 0 for no limit).
 
 Hot Pages
 .........
-- 
2.52.0

Re: [PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[Pierrick Bouvier]

On 12/2/25 3:05 PM, Alex Bradbury wrote:
> This is a resend of my previous patchset which unfortunately seems not to have
> been applied (see
> <https://lore.kernel.org/qemu-devel/cover.1753857212.git.asb@igalia.com/).
> I've rebased on current HEAD, checked it works as expected, and added
> Reviewed-by tags to the patches, which all received review.
>

Sorry about this, this unfortunately happens with email based development.

@alex.bennee, is that still on time to merge this for next rc?
The changes are trivial, and located in this plugin file only.

Regards,
Pierrick

Re: [PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[Alex Bradbury]

On 2025-12-02 23:05, Alex Bradbury wrote:
> This is a resend of my previous patchset which unfortunately seems not to have
> been applied (see
> <https://lore.kernel.org/qemu-devel/cover.1753857212.git.asb@igalia.com/).
> I've rebased on current HEAD, checked it works as expected, and added
> Reviewed-by tags to the patches, which all received review.
> 
> Repeating the summary from last time:
> 
> This series contains one minor feature addition and a series of small
> bugfixes/improvements. The addition that motivates the submission is to add a
> limit argument for the hotblocks plugin, allowing you to control how many
> blocks are printed rather than being hardcoded to the 20 most executed.
> Setting limit=0 and dumping information about all executed blocks is
> incredibly helpful for an analysis script I have downstream.
> 
> This is my first contribution to QEMU. I've attempted to follow all of the
> guidance in the "Submitting a Patch" guide, but apologies if I missed
> anything.
> 
> Alex Bradbury (5):
>   contrib/plugins/hotblocks: Correctly free sorted counts list
>   contrib/plugins/hotblocks: Fix off by one error in iteration of sorted
>     blocks
>   contrib/plugins/hotblocks: Print uint64_t with PRIu64 rather than
>     PRId64
>   docs/about/emulation: Add documentation for hotblocks plugin arguments
>   contrib/plugins/hotblocks: Allow limit to be set as a command line
>     argument
> 
>  contrib/plugins/hotblocks.c | 20 ++++++++++++++------
>  docs/about/emulation.rst    | 12 ++++++++++++
>  2 files changed, 26 insertions(+), 6 deletions(-)

Sending a ping as per the guidance at
<https://www.qemu.org/docs/master/devel/submitting-a-patch.html#if-your-patch-seems-to-have-been-ignored>,
and also linking to the patch on lore.kernel.org as that page suggests
<https://lore.kernel.org/qemu-devel/cover.1764716538.git.asb@igalia.com/>

Thanks.

Alex

Re: [PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[Alex Bennée]

Alex Bradbury <asb@igalia.com> writes:

> On 2025-12-02 23:05, Alex Bradbury wrote:
>> This is a resend of my previous patchset which unfortunately seems not to have
>> been applied (see
>> <https://lore.kernel.org/qemu-devel/cover.1753857212.git.asb@igalia.com/).
>> I've rebased on current HEAD, checked it works as expected, and added
>> Reviewed-by tags to the patches, which all received review.
>> 
>> Repeating the summary from last time:
>> 
>> This series contains one minor feature addition and a series of small
>> bugfixes/improvements. The addition that motivates the submission is to add a
>> limit argument for the hotblocks plugin, allowing you to control how many
>> blocks are printed rather than being hardcoded to the 20 most executed.
>> Setting limit=0 and dumping information about all executed blocks is
>> incredibly helpful for an analysis script I have downstream.
>> 
>> This is my first contribution to QEMU. I've attempted to follow all of the
>> guidance in the "Submitting a Patch" guide, but apologies if I missed
>> anything.
>> 
>> Alex Bradbury (5):
>>   contrib/plugins/hotblocks: Correctly free sorted counts list
>>   contrib/plugins/hotblocks: Fix off by one error in iteration of sorted
>>     blocks
>>   contrib/plugins/hotblocks: Print uint64_t with PRIu64 rather than
>>     PRId64
>>   docs/about/emulation: Add documentation for hotblocks plugin arguments
>>   contrib/plugins/hotblocks: Allow limit to be set as a command line
>>     argument
>> 
>>  contrib/plugins/hotblocks.c | 20 ++++++++++++++------
>>  docs/about/emulation.rst    | 12 ++++++++++++
>>  2 files changed, 26 insertions(+), 6 deletions(-)
>
> Sending a ping as per the guidance at
> <https://www.qemu.org/docs/master/devel/submitting-a-patch.html#if-your-patch-seems-to-have-been-ignored>,
> and also linking to the patch on lore.kernel.org as that page suggests
> <https://lore.kernel.org/qemu-devel/cover.1764716538.git.asb@igalia.com/>

Sorry I missed this in v1. As we are getting very close to 10.2 getting
out of the door I'll punt this until the tree re-opens. I have it on my
backlog now.

If you want I can also cc qemu-stable if you think its worth having the
cleanups in 10.2.1

>
> Thanks.
>
> Alex

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 0/5] contrib/plugins/hotblocks: Minor bug fixes and add limit argument

[Alex Bennée]

Alex Bradbury <asb@igalia.com> writes:

> This is a resend of my previous patchset which unfortunately seems not to have
> been applied (see
> <https://lore.kernel.org/qemu-devel/cover.1753857212.git.asb@igalia.com/).
> I've rebased on current HEAD, checked it works as expected, and added
> Reviewed-by tags to the patches, which all received review.

Queued to plugins/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro