[lttng-dev] [RFC PATCH babeltrace] ctf-writer: Add support for the cpu_id field

Jérémie Galarneau jeremie.galarneau at efficios.com
Mon Jun 2 13:08:21 EDT 2014


Hi Sebastian,

First of all, thanks for this patch! Comments follow.

On Tue, May 27, 2014 at 4:08 AM, Sebastian Andrzej Siewior
<bigeasy at linutronix.de> wrote:
> This patch attempts to add support for cpu_id which could be recorded
> within the trace. The lttng tracer provides a CPU field in its traces
> and this aims to offer the same functionality and the same file format.
> The way this is implemented right now it requires to flush the stream
> each time the CPU id has been changed. What might work as well is to
> assign each CPU to a specific stream.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> ---
>  bindings/python/babeltrace.i.in                 |  8 +++++++
>  formats/ctf/writer/stream.c                     | 28 +++++++++++++++++++++++++
>  include/babeltrace/ctf-writer/stream-internal.h |  1 +
>  include/babeltrace/ctf-writer/stream.h          | 12 +++++++++++
>  4 files changed, 49 insertions(+)
>
> diff --git a/bindings/python/babeltrace.i.in b/bindings/python/babeltrace.i.in
> index 0f81c40..ca443fc 100644
> --- a/bindings/python/babeltrace.i.in
> +++ b/bindings/python/babeltrace.i.in
> @@ -1364,6 +1364,7 @@ void bt_ctf_event_put(struct bt_ctf_event *event);
>  %rename("_bt_ctf_stream_class_get") bt_ctf_stream_class_get(struct bt_ctf_stream_class *stream_class);
>  %rename("_bt_ctf_stream_class_put") bt_ctf_stream_class_put(struct bt_ctf_stream_class *stream_class);
>  %rename("_bt_ctf_stream_append_discarded_events") bt_ctf_stream_append_discarded_events(struct bt_ctf_stream *stream, uint64_t event_count);
> +%rename("_bt_ctf_stream_append_cpu_id") bt_ctf_stream_append_cpu_id(struct bt_ctf_stream *stream, uint64_t cpu_id);
>  %rename("_bt_ctf_stream_append_event") bt_ctf_stream_append_event(struct bt_ctf_stream *stream, struct bt_ctf_event *event);
>  %rename("_bt_ctf_stream_flush") bt_ctf_stream_flush(struct bt_ctf_stream *stream);
>  %rename("_bt_ctf_stream_get") bt_ctf_stream_get(struct bt_ctf_stream *stream);
> @@ -1375,6 +1376,7 @@ int bt_ctf_stream_class_add_event_class(struct bt_ctf_stream_class *stream_class
>  void bt_ctf_stream_class_get(struct bt_ctf_stream_class *stream_class);
>  void bt_ctf_stream_class_put(struct bt_ctf_stream_class *stream_class);
>  void bt_ctf_stream_append_discarded_events(struct bt_ctf_stream *stream, uint64_t event_count);
> +void bt_ctf_stream_append_cpu_id(struct bt_ctf_stream *stream, uint64_t cpu_id);
>  int bt_ctf_stream_append_event(struct bt_ctf_stream *stream, struct bt_ctf_event *event);
>  int bt_ctf_stream_flush(struct bt_ctf_stream *stream);
>  void bt_ctf_stream_get(struct bt_ctf_stream *stream);
> @@ -2103,6 +2105,12 @@ void bt_ctf_writer_put(struct bt_ctf_writer *writer);
>                         _bt_ctf_stream_append_discarded_events(self._s, event_count)
>
>                 """
> +               Set the current CPU for the events that occured.
> +               """
> +               def append_cpu_id(self, cpu_id):
> +                       _bt_ctf_stream_append_cpu_id(self._s, cpu_id)
> +
> +               """
>                 Append "event" to the stream's current packet. The stream's associated clock
>                 will be sampled during this call. The event shall not be modified after
>                 being appended to a stream.
> diff --git a/formats/ctf/writer/stream.c b/formats/ctf/writer/stream.c
> index 4efb369..86eed3f 100644
> --- a/formats/ctf/writer/stream.c
> +++ b/formats/ctf/writer/stream.c
> @@ -335,6 +335,16 @@ void bt_ctf_stream_append_discarded_events(struct bt_ctf_stream *stream,
>         stream->events_discarded += event_count;
>  }
>
> +void bt_ctf_stream_append_cpu_id(struct bt_ctf_stream *stream,
> +               uint64_t cpu_id)
> +{
> +       if (!stream) {
> +               return;
> +       }
> +
> +       stream->cpu_id = cpu_id;
> +}
> +
>  int bt_ctf_stream_append_event(struct bt_ctf_stream *stream,
>                 struct bt_ctf_event *event)
>  {
> @@ -409,6 +419,12 @@ int bt_ctf_stream_flush(struct bt_ctf_stream *stream)
>         }
>
>         ret = set_structure_field_integer(stream_class->packet_context,
> +               "cpu_id", stream->cpu_id);
> +       if (ret) {
> +               goto end;
> +       }
> +
> +       ret = set_structure_field_integer(stream_class->packet_context,
>                 "content_size", UINT64_MAX);
>         if (ret) {
>                 goto end;
> @@ -623,6 +639,8 @@ int init_packet_context(struct bt_ctf_stream_class *stream_class,
>                 bt_ctf_field_type_structure_create();
>         struct bt_ctf_field_type *_uint64_t =
>                 get_field_type(FIELD_TYPE_ALIAS_UINT64_T);
> +       struct bt_ctf_field_type *_uint32_t =
> +               get_field_type(FIELD_TYPE_ALIAS_UINT32_T);
>
>         if (!packet_context_type) {
>                 ret = -1;
> @@ -637,6 +655,10 @@ int init_packet_context(struct bt_ctf_stream_class *stream_class,
>         if (ret) {
>                 goto end;
>         }
> +       ret = bt_ctf_field_type_set_byte_order(_uint32_t, byte_order);
> +       if (ret) {
> +               goto end;
> +       }
>
>         ret = bt_ctf_field_type_structure_add_field(packet_context_type,
>                 _uint64_t, "timestamp_begin");
> @@ -667,6 +689,11 @@ int init_packet_context(struct bt_ctf_stream_class *stream_class,
>         if (ret) {
>                 goto end;
>         }
> +       ret = bt_ctf_field_type_structure_add_field(packet_context_type,
> +               _uint32_t, "cpu_id");
> +       if (ret) {
> +               goto end;
> +       }
>
>         stream_class->packet_context_type = packet_context_type;
>         stream_class->packet_context = bt_ctf_field_create(packet_context_type);
> @@ -680,6 +707,7 @@ int init_packet_context(struct bt_ctf_stream_class *stream_class,
>         }
>
>         bt_ctf_field_type_put(_uint64_t);
> +       bt_ctf_field_type_put(_uint32_t);
>         return ret;
>  }
>
> diff --git a/include/babeltrace/ctf-writer/stream-internal.h b/include/babeltrace/ctf-writer/stream-internal.h
> index 57dd992..8c14166 100644
> --- a/include/babeltrace/ctf-writer/stream-internal.h
> +++ b/include/babeltrace/ctf-writer/stream-internal.h
> @@ -69,6 +69,7 @@ struct bt_ctf_stream {
>         GPtrArray *events;
>         struct ctf_stream_pos pos;
>         unsigned int flushed_packet_count;
> +       uint32_t cpu_id;
>         uint64_t events_discarded;
>  };
>
> diff --git a/include/babeltrace/ctf-writer/stream.h b/include/babeltrace/ctf-writer/stream.h
> index 22fb697..2f49810 100644
> --- a/include/babeltrace/ctf-writer/stream.h
> +++ b/include/babeltrace/ctf-writer/stream.h
> @@ -116,6 +116,18 @@ extern void bt_ctf_stream_append_discarded_events(struct bt_ctf_stream *stream,
>                 uint64_t event_count);
>
>  /*
> + * bt_ctf_stream_append_cpu_id: set the CPU for the events.
> + *
> + * Set the CPU number on which the events occured.
> + *
> + * @param stream Stream instance.
> + * @param cpu_id CPU number for the event to record.
> + *
> + */
> +extern void bt_ctf_stream_append_cpu_id(struct bt_ctf_stream *stream,
> +               uint64_t cpu_id);
> +

I'd prefer we provide a generic way of adding fields to the packet
context. Something akin to what's provided for structures which would
let users define arbitrary packet context fields.

I propose adding

int bt_ctf_stream_class_add_packet_context_field(const char
*field_name, struct bt_ctf_field_type *field_type);

and

struct bt_ctf_field *bt_ctf_stream_get_packet_context_field(const char
*field_name);

I'm open to better naming suggestions, but hopefully I'm getting the
functionality across.

Thoughts?

Jérémie

> +/*
>   * bt_ctf_stream_append_event: append an event to the stream.
>   *
>   * Append "event" to the stream's current packet. The stream's associated clock
> --
> 2.0.0.rc4
>
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com



More information about the lttng-dev mailing list