Hi Sebastian, First of all, thanks for this patch! Comments follow.
On Tue, May 27, 2014 at 4:08 AM, Sebastian Andrzej Siewior <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
