[Qemu-devel] [PATCH] trace: forbid floating point types

[Qemu-devel] [PATCH] trace: forbid floating point types

[Stefan Hajnoczi]

Only one existing trace event uses a floating point type.  Unfortunately
float and double cannot be supported since SystemTap does not have
floating point types.

Remove float and double from the whitelist and document this limitation.
Update the migrate_transferred trace event to use uint64_t instead of
double.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/tracing.txt        | 5 +++++
 migration/trace-events        | 2 +-
 qapi/trace-events             | 2 +-
 scripts/tracetool/__init__.py | 2 --
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 07abbb345c..6f815ecbd7 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -104,6 +104,11 @@ Trace events should use types as follows:
  * For everything else, use primitive scalar types (char, int, long) with the
    appropriate signedness.
 
+ * Avoid floating point types (float and double) because SystemTap does not
+   support them.  In most cases it is possible to round to an integer type
+   instead.  This may require scaling the value first by multiplying it by 1000
+   or the like when digits after the decimal point need to be preserved.
+
 Format strings should reflect the types defined in the trace event.  Take
 special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
 respectively.  This ensures portability between 32- and 64-bit platforms.
diff --git a/migration/trace-events b/migration/trace-events
index 3f67758893..7ea522e453 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -133,7 +133,7 @@ migrate_global_state_post_load(const char *state) "loaded state: %s"
 migrate_global_state_pre_save(const char *state) "saved state: %s"
 migration_thread_low_pending(uint64_t pending) "%" PRIu64
 migrate_state_too_big(void) ""
-migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
+migrate_transferred(uint64_t tranferred, uint64_t time_spent, uint64_t bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %" PRIu64 " max_size %" PRId64
 process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
 process_incoming_migration_co_postcopy_end_main(void) ""
 migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
diff --git a/qapi/trace-events b/qapi/trace-events
index 9e9008a1dc..70e049ea80 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -29,6 +29,6 @@ visit_type_int64(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
 visit_type_size(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
 visit_type_bool(void *v, const char *name, bool *obj) "v=%p name=%s obj=%p"
 visit_type_str(void *v, const char *name, char **obj) "v=%p name=%s obj=%p"
-visit_type_number(void *v, const char *name, double *obj) "v=%p name=%s obj=%p"
+visit_type_number(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
 visit_type_any(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
 visit_type_null(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index b20fac34a3..0e3c9e146c 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -53,8 +53,6 @@ ALLOWED_TYPES = [
     "bool",
     "unsigned",
     "signed",
-    "float",
-    "double",
     "int8_t",
     "uint8_t",
     "int16_t",
-- 
2.17.1

Re: [Qemu-devel] [PATCH] trace: forbid floating point types

[Philippe Mathieu-Daudé]

On 06/21/2018 12:02 PM, Stefan Hajnoczi wrote:
> Only one existing trace event uses a floating point type.  Unfortunately
> float and double cannot be supported since SystemTap does not have
> floating point types.
> 
> Remove float and double from the whitelist and document this limitation.
> Update the migrate_transferred trace event to use uint64_t instead of
> double.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  docs/devel/tracing.txt        | 5 +++++
>  migration/trace-events        | 2 +-
>  qapi/trace-events             | 2 +-
>  scripts/tracetool/__init__.py | 2 --
>  4 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
> index 07abbb345c..6f815ecbd7 100644
> --- a/docs/devel/tracing.txt
> +++ b/docs/devel/tracing.txt
> @@ -104,6 +104,11 @@ Trace events should use types as follows:
>   * For everything else, use primitive scalar types (char, int, long) with the
>     appropriate signedness.
>  
> + * Avoid floating point types (float and double) because SystemTap does not
> +   support them.  In most cases it is possible to round to an integer type
> +   instead.  This may require scaling the value first by multiplying it by 1000
> +   or the like when digits after the decimal point need to be preserved.
> +
>  Format strings should reflect the types defined in the trace event.  Take
>  special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
>  respectively.  This ensures portability between 32- and 64-bit platforms.
> diff --git a/migration/trace-events b/migration/trace-events
> index 3f67758893..7ea522e453 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -133,7 +133,7 @@ migrate_global_state_post_load(const char *state) "loaded state: %s"
>  migrate_global_state_pre_save(const char *state) "saved state: %s"
>  migration_thread_low_pending(uint64_t pending) "%" PRIu64
>  migrate_state_too_big(void) ""
> -migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
> +migrate_transferred(uint64_t tranferred, uint64_t time_spent, uint64_t bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %" PRIu64 " max_size %" PRId64
>  process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
>  process_incoming_migration_co_postcopy_end_main(void) ""
>  migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
> diff --git a/qapi/trace-events b/qapi/trace-events
> index 9e9008a1dc..70e049ea80 100644
> --- a/qapi/trace-events
> +++ b/qapi/trace-events
> @@ -29,6 +29,6 @@ visit_type_int64(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
>  visit_type_size(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
>  visit_type_bool(void *v, const char *name, bool *obj) "v=%p name=%s obj=%p"
>  visit_type_str(void *v, const char *name, char **obj) "v=%p name=%s obj=%p"
> -visit_type_number(void *v, const char *name, double *obj) "v=%p name=%s obj=%p"
> +visit_type_number(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
>  visit_type_any(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
>  visit_type_null(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index b20fac34a3..0e3c9e146c 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -53,8 +53,6 @@ ALLOWED_TYPES = [
>      "bool",
>      "unsigned",
>      "signed",
> -    "float",
> -    "double",
>      "int8_t",
>      "uint8_t",
>      "int16_t",
> 

Re: [Qemu-devel] [PATCH] trace: forbid floating point types

[Juan Quintela]

Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Only one existing trace event uses a floating point type.  Unfortunately
> float and double cannot be supported since SystemTap does not have
> floating point types.
>
> Remove float and double from the whitelist and document this limitation.
> Update the migrate_transferred trace event to use uint64_t instead of
> double.
>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

We don't really care so much about this value, and or how it is printed.

Re: [Qemu-devel] [PATCH] trace: forbid floating point types

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]

On Thu, Jun 21, 2018 at 04:02:54PM +0100, Stefan Hajnoczi wrote:
> Only one existing trace event uses a floating point type.  Unfortunately
> float and double cannot be supported since SystemTap does not have
> floating point types.
> 
> Remove float and double from the whitelist and document this limitation.
> Update the migrate_transferred trace event to use uint64_t instead of
> double.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  docs/devel/tracing.txt        | 5 +++++
>  migration/trace-events        | 2 +-
>  qapi/trace-events             | 2 +-
>  scripts/tracetool/__init__.py | 2 --
>  4 files changed, 7 insertions(+), 4 deletions(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]