[RFC PATCH] contrib/plugins: avoid hanging program

Alex Bennée posted 1 patch 2 months, 1 week ago
contrib/plugins/ips.c | 5 +++++
1 file changed, 5 insertions(+)
[RFC PATCH] contrib/plugins: avoid hanging program
Posted by Alex Bennée 2 months, 1 week ago
Although we asks for instructions per second we work in quanta and
that cannot be 0. Fail to load the plugin instead and report the
minimum IPS we can handle.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Elisha Hollander <just4now666666@gmail.com>
---
 contrib/plugins/ips.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
index 29fa556d0f..6f078689dc 100644
--- a/contrib/plugins/ips.c
+++ b/contrib/plugins/ips.c
@@ -152,6 +152,11 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
     vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
     max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
 
+    if (max_insn_per_quantum == 0) {
+        fprintf(stderr, "minimum of %d instructions per second needed\n", NUM_TIME_UPDATE_PER_SEC);
+        return -1;
+    }
+
     time_handle = qemu_plugin_request_time_control();
     g_assert(time_handle);
 
-- 
2.39.2


Re: [RFC PATCH] contrib/plugins: avoid hanging program
Posted by Pierrick Bouvier 2 months ago
On 9/13/24 10:38, Alex Bennée wrote:
> Although we asks for instructions per second we work in quanta and
> that cannot be 0. Fail to load the plugin instead and report the
> minimum IPS we can handle.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Elisha Hollander <just4now666666@gmail.com>
> ---
>   contrib/plugins/ips.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> index 29fa556d0f..6f078689dc 100644
> --- a/contrib/plugins/ips.c
> +++ b/contrib/plugins/ips.c
> @@ -152,6 +152,11 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>       vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
>       max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
>   
> +    if (max_insn_per_quantum == 0) {
> +        fprintf(stderr, "minimum of %d instructions per second needed\n", NUM_TIME_UPDATE_PER_SEC);
> +        return -1;
> +    }
> +
>       time_handle = qemu_plugin_request_time_control();
>       g_assert(time_handle);
>   

The ips plugin uses tb based trap, so it should not be expected that an 
ips of 1 will do what is expected (it will execute the whole tb at minimum).

Either we could extend this plugin with a precise mode (instruction 
based), or simply emit an error when ips is too low, like < 100.
I'm more in favor of the latter solution.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Re: [RFC PATCH] contrib/plugins: avoid hanging program
Posted by Richard Henderson 2 months, 1 week ago
On 9/13/24 10:38, Alex Bennée wrote:
> Although we asks for instructions per second we work in quanta and
> that cannot be 0. Fail to load the plugin instead and report the
> minimum IPS we can handle.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Reported-by: Elisha Hollander<just4now666666@gmail.com>
> ---
>   contrib/plugins/ips.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~