| Message ID | 20250506125715.232872-6-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Maintainer updates for May (testing, plugins, virtio-gpu) | expand |
On 2025/05/06 21:57, Alex Bennée wrote: > It's easy to get lost in zeros while setting the numbers of > instructions per second. Add a scaling suffix to make things simpler. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > --- > v2 > - normalise the suffix before a full strcmp0 > - check endptr actually set > - fix checkpatch > --- > contrib/plugins/ips.c | 36 +++++++++++++++++++++++++++++++++++- > 1 file changed, 35 insertions(+), 1 deletion(-) > > diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c > index e5297dbb01..9b166a7d6c 100644 > --- a/contrib/plugins/ips.c > +++ b/contrib/plugins/ips.c > @@ -20,6 +20,8 @@ > > QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; > > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > + G_N_ELEMENTS() is already available. > /* how many times do we update time per sec */ > #define NUM_TIME_UPDATE_PER_SEC 10 > #define NSEC_IN_ONE_SEC (1000 * 1000 * 1000) > @@ -129,6 +131,18 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata) > qemu_plugin_scoreboard_free(vcpus); > } > > +typedef struct { > + const char *suffix; > + unsigned long multipler; I prefer to have an explicitly-sized type: uint32_t in this case. It also saves typing several characters as a bonus. > +} scale_entry; docs/devel/style.rst says > Structured type names are in CamelCase; harder to type but standing > out. > + > +/* a bit like units.h but not binary */ > +static scale_entry scales[] = { > + { "khz", 1000 }, > + { "mhz", 1000 * 1000 }, > + { "ghz", 1000 * 1000 * 1000 }, Having "hz" as suffixes look a bit awkard. "1 giga instructions per second" sounds natural, but "1 gigahertz instructions per second" doesn't to me. Practically, it would be easier to just type "g" instead of "ghz". util/cutils.c has similar code though I guess a plugin cannot be linked to it. > +}; > + > QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, > const qemu_info_t *info, int argc, > char **argv) > @@ -137,12 +151,32 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, > char *opt = argv[i]; > g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); > if (g_strcmp0(tokens[0], "ips") == 0) { > - max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10); > + char *endptr = NULL; > + max_insn_per_second = g_ascii_strtoull(tokens[1], &endptr, 10); > if (!max_insn_per_second && errno) { > fprintf(stderr, "%s: couldn't parse %s (%s)\n", > __func__, tokens[1], g_strerror(errno)); > return -1; > } > + > + if (endptr && *endptr != 0) { > + g_autofree gchar *lower = g_utf8_strdown(endptr, -1); > + unsigned long scale = 0; > + > + for (int j = 0; j < ARRAY_SIZE(scales); j++) { > + if (g_strcmp0(lower, scales[j].suffix) == 0) { > + scale = scales[j].multipler; > + break; > + } > + } > + > + if (scale) { > + max_insn_per_second *= scale; > + } else { > + fprintf(stderr, "bad suffix: %s\n", endptr); > + return -1; > + } > + } > } else { > fprintf(stderr, "option parsing failed: %s\n", opt); > return -1;
diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c index e5297dbb01..9b166a7d6c 100644 --- a/contrib/plugins/ips.c +++ b/contrib/plugins/ips.c @@ -20,6 +20,8 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION; +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + /* how many times do we update time per sec */ #define NUM_TIME_UPDATE_PER_SEC 10 #define NSEC_IN_ONE_SEC (1000 * 1000 * 1000) @@ -129,6 +131,18 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata) qemu_plugin_scoreboard_free(vcpus); } +typedef struct { + const char *suffix; + unsigned long multipler; +} scale_entry; + +/* a bit like units.h but not binary */ +static scale_entry scales[] = { + { "khz", 1000 }, + { "mhz", 1000 * 1000 }, + { "ghz", 1000 * 1000 * 1000 }, +}; + QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, int argc, char **argv) @@ -137,12 +151,32 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, char *opt = argv[i]; g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "ips") == 0) { - max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10); + char *endptr = NULL; + max_insn_per_second = g_ascii_strtoull(tokens[1], &endptr, 10); if (!max_insn_per_second && errno) { fprintf(stderr, "%s: couldn't parse %s (%s)\n", __func__, tokens[1], g_strerror(errno)); return -1; } + + if (endptr && *endptr != 0) { + g_autofree gchar *lower = g_utf8_strdown(endptr, -1); + unsigned long scale = 0; + + for (int j = 0; j < ARRAY_SIZE(scales); j++) { + if (g_strcmp0(lower, scales[j].suffix) == 0) { + scale = scales[j].multipler; + break; + } + } + + if (scale) { + max_insn_per_second *= scale; + } else { + fprintf(stderr, "bad suffix: %s\n", endptr); + return -1; + } + } } else { fprintf(stderr, "option parsing failed: %s\n", opt); return -1;