On 2/29/24 2:08 AM, Alex Bennée wrote:
Luc Michel <luc.mic...@amd.com> writes:Hi Pierrick, On 13:14 Mon 26 Feb , Pierrick Bouvier wrote:Signed-off-by: Pierrick Bouvier <pierrick.bouv...@linaro.org> --- tests/plugin/mem.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c index 44e91065ba7..d4729f5e015 100644 --- a/tests/plugin/mem.c +++ b/tests/plugin/mem.c @@ -16,9 +16,14 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; -static uint64_t inline_mem_count; -static uint64_t cb_mem_count; -static uint64_t io_count; +typedef struct { + uint64_t mem_count; + uint64_t io_count; +} CPUCount; + +static struct qemu_plugin_scoreboard *counts; +static qemu_plugin_u64 mem_count; +static qemu_plugin_u64 io_count;I see that you merged inline and callback counts into the same variable. I wonder... For this test don't you want to keep a plain uint64_t for callback counts? I have the feeling that this test was made so one can make sure inline and callback counts match.Indeed the problem now is double counting: ➜ ./qemu-hppa -plugin ./tests/plugin/libmem.so,inline=true -d plugin ./tests/tcg/hppa-linux-user/sha512 1..10 ok 1 - do_test(&tests[i]) ok 2 - do_test(&tests[i]) ok 3 - do_test(&tests[i]) ok 4 - do_test(&tests[i]) ok 5 - do_test(&tests[i]) ok 6 - do_test(&tests[i]) ok 7 - do_test(&tests[i]) ok 8 - do_test(&tests[i]) ok 9 - do_test(&tests[i]) ok 10 - do_test(&tests[i]) mem accesses: 262917 🕙22:06:57 alex@draig:qemu.git/builds/all on plugins/next [$?] ➜ ./qemu-hppa -plugin ./tests/plugin/libmem.so,inline=true,callback=true -d plugin ./tests/tcg/hppa-linux-user/sha512 1..10 ok 1 - do_test(&tests[i]) ok 2 - do_test(&tests[i]) ok 3 - do_test(&tests[i]) ok 4 - do_test(&tests[i]) ok 5 - do_test(&tests[i]) ok 6 - do_test(&tests[i]) ok 7 - do_test(&tests[i]) ok 8 - do_test(&tests[i]) ok 9 - do_test(&tests[i]) ok 10 - do_test(&tests[i]) mem accesses: 525834 although perhaps it would just be simpler for the plugin to only accept one or the other method.
My bad. Other plugins enable only inline when both are supplied, so I missed this here. I added an explicit error when user enable callback and inline at the same time on this plugin.
Lucstatic bool do_inline, do_callback; static bool do_haddr; static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW; @@ -27,16 +32,16 @@ static void plugin_exit(qemu_plugin_id_t id, void *p) { g_autoptr(GString) out = g_string_new(""); - if (do_inline) { - g_string_printf(out, "inline mem accesses: %" PRIu64 "\n", inline_mem_count); - } - if (do_callback) { - g_string_append_printf(out, "callback mem accesses: %" PRIu64 "\n", cb_mem_count); + if (do_inline || do_callback) { + g_string_printf(out, "mem accesses: %" PRIu64 "\n", + qemu_plugin_u64_sum(mem_count)); } if (do_haddr) { - g_string_append_printf(out, "io accesses: %" PRIu64 "\n", io_count); + g_string_append_printf(out, "io accesses: %" PRIu64 "\n", + qemu_plugin_u64_sum(io_count)); } qemu_plugin_outs(out->str); + qemu_plugin_scoreboard_free(counts); } static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo, @@ -46,12 +51,12 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo, struct qemu_plugin_hwaddr *hwaddr; hwaddr = qemu_plugin_get_hwaddr(meminfo, vaddr); if (qemu_plugin_hwaddr_is_io(hwaddr)) { - io_count++; + qemu_plugin_u64_add(io_count, cpu_index, 1); } else { - cb_mem_count++; + qemu_plugin_u64_add(mem_count, cpu_index, 1); } } else { - cb_mem_count++; + qemu_plugin_u64_add(mem_count, cpu_index, 1); } } @@ -64,9 +69,10 @@ 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_mem_inline(insn, rw, - QEMU_PLUGIN_INLINE_ADD_U64, - &inline_mem_count, 1); + qemu_plugin_register_vcpu_mem_inline_per_vcpu( + insn, rw, + QEMU_PLUGIN_INLINE_ADD_U64, + mem_count, 1); } if (do_callback) { qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem, @@ -117,6 +123,10 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, } } + counts = qemu_plugin_scoreboard_new(sizeof(CPUCount)); + mem_count = qemu_plugin_scoreboard_u64_in_struct( + counts, CPUCount, mem_count); + io_count = qemu_plugin_scoreboard_u64_in_struct(counts, CPUCount, io_count); qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans); qemu_plugin_register_atexit_cb(id, plugin_exit, NULL); return 0; -- 2.43.0
