Show message when errors occurred during ctf conversion setup.
Before this patch:
$ ./perf data convert --to-ctf=ctf
$ echo $?
255
After this patch:
$ ./perf data convert --to-ctf=ctf
Error during CTF convert setup.
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/data-convert-bt.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index dd17c9a..a5b89b9 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -847,11 +847,15 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
(double) c.events_size / 1024.0 / 1024.0,
c.events_count);
- /* its all good */
-free_session:
perf_session__delete(session);
+ ctf_writer__cleanup(cw);
+
+ return err;
+free_session:
+ perf_session__delete(session);
free_writer:
ctf_writer__cleanup(cw);
+ pr_err("Error during CTF convert setup.\n");
return err;
}
--
2.3.3.220.g9ab698f
Due to babeltrace commit:
7f800dc7c2a1 ("ir: make trace environment use bt_object")
The trace->frozen flag is set in bt_ctf_trace_create_stream(), this flag
is checked before adding environment field to trace, and causes
ctf_writer__setup_env() failed. Fix this by setting all environment
fields before bt_ctf_trace_create_stream().
Before this patch:
$ perf data convert --to-ctf=ctf
Error during CTF convert setup.
After this patch:
$ perf data convert --to-ctf=ctf
[ perf data convert: Converted 'perf.data' into CTF data 'ctf' ]
[ perf data convert: Converted and wrote 0.023 MB (596 samples) ]
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/data-convert-bt.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index a5b89b9..718dc8a 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -604,6 +604,22 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
return 0;
}
+static int ctf_writer__setup_stream(struct ctf_writer *cw)
+{
+ struct bt_ctf_stream *stream;
+
+ /* CTF stream instance */
+ stream = bt_ctf_writer_create_stream(cw->writer, cw->stream_class);
+ if (!stream) {
+ pr("Failed to create CTF stream.\n");
+ return -1;
+ }
+
+ cw->stream = stream;
+
+ return 0;
+}
+
static int ctf_writer__setup_env(struct ctf_writer *cw,
struct perf_session *session)
{
@@ -725,7 +741,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
{
struct bt_ctf_writer *writer;
struct bt_ctf_stream_class *stream_class;
- struct bt_ctf_stream *stream;
struct bt_ctf_clock *clock;
/* CTF writer */
@@ -767,15 +782,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
if (ctf_writer__init_data(cw))
goto err_cleanup;
- /* CTF stream instance */
- stream = bt_ctf_writer_create_stream(writer, stream_class);
- if (!stream) {
- pr("Failed to create CTF stream.\n");
- goto err_cleanup;
- }
-
- cw->stream = stream;
-
/* CTF clock writer setup */
if (bt_ctf_writer_add_clock(writer, clock)) {
pr("Failed to assign CTF clock to writer.\n");
@@ -830,6 +836,10 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
if (ctf_writer__setup_env(cw, session))
goto free_session;
+ /* CTF writer trace stream setup */
+ if (ctf_writer__setup_stream(cw))
+ goto free_session;
+
/* CTF events setup */
if (setup_events(cw, session))
goto free_session;
--
2.3.3.220.g9ab698f
On Wed, Apr 08, 2015 at 12:49:19PM +0800, He Kuang wrote:
> Show message when errors occurred during ctf conversion setup.
>
> Before this patch:
> $ ./perf data convert --to-ctf=ctf
> $ echo $?
> 255
>
> After this patch:
> $ ./perf data convert --to-ctf=ctf
> Error during CTF convert setup.
so I have like 5 more patches from the original CTF set
which I'm holding until all works with tracecompass:
http://marc.info/?l=linux-kernel&m=142736197610573&w=2
Is it working for you? How do you test resulted CTF data?
anyway the patch looks ok, just small nit below
>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/data-convert-bt.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index dd17c9a..a5b89b9 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -847,11 +847,15 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
> (double) c.events_size / 1024.0 / 1024.0,
> c.events_count);
>
> - /* its all good */
> -free_session:
> perf_session__delete(session);
> + ctf_writer__cleanup(cw);
> +
this leg can also fail due to:
err = perf_session__process_events(session);
if (!err)
err = bt_ctf_stream_flush(cw->stream);
so we might want to inform about that like:
if (err)
pr_err("Error during conversion.\n");
thanks,
jirka
> + return err;
>
> +free_session:
> + perf_session__delete(session);
> free_writer:
> ctf_writer__cleanup(cw);
> + pr_err("Error during CTF convert setup.\n");
> return err;
> }
> --
> 2.3.3.220.g9ab698f
On Wed, Apr 08, 2015 at 12:49:20PM +0800, He Kuang wrote:
> Due to babeltrace commit:
> 7f800dc7c2a1 ("ir: make trace environment use bt_object")
>
> The trace->frozen flag is set in bt_ctf_trace_create_stream(), this flag
> is checked before adding environment field to trace, and causes
> ctf_writer__setup_env() failed. Fix this by setting all environment
> fields before bt_ctf_trace_create_stream().
>
> Before this patch:
> $ perf data convert --to-ctf=ctf
> Error during CTF convert setup.
have you tested with the latest babeltrace sources?
this reminds me the bug they fixed recently, CCing Jeremie
thanks,
jirka
>
> After this patch:
> $ perf data convert --to-ctf=ctf
> [ perf data convert: Converted 'perf.data' into CTF data 'ctf' ]
> [ perf data convert: Converted and wrote 0.023 MB (596 samples) ]
>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/data-convert-bt.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index a5b89b9..718dc8a 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -604,6 +604,22 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
> return 0;
> }
>
> +static int ctf_writer__setup_stream(struct ctf_writer *cw)
> +{
> + struct bt_ctf_stream *stream;
> +
> + /* CTF stream instance */
> + stream = bt_ctf_writer_create_stream(cw->writer, cw->stream_class);
> + if (!stream) {
> + pr("Failed to create CTF stream.\n");
> + return -1;
> + }
> +
> + cw->stream = stream;
> +
> + return 0;
> +}
> +
> static int ctf_writer__setup_env(struct ctf_writer *cw,
> struct perf_session *session)
> {
> @@ -725,7 +741,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
> {
> struct bt_ctf_writer *writer;
> struct bt_ctf_stream_class *stream_class;
> - struct bt_ctf_stream *stream;
> struct bt_ctf_clock *clock;
>
> /* CTF writer */
> @@ -767,15 +782,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
> if (ctf_writer__init_data(cw))
> goto err_cleanup;
>
> - /* CTF stream instance */
> - stream = bt_ctf_writer_create_stream(writer, stream_class);
> - if (!stream) {
> - pr("Failed to create CTF stream.\n");
> - goto err_cleanup;
> - }
> -
> - cw->stream = stream;
> -
> /* CTF clock writer setup */
> if (bt_ctf_writer_add_clock(writer, clock)) {
> pr("Failed to assign CTF clock to writer.\n");
> @@ -830,6 +836,10 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
> if (ctf_writer__setup_env(cw, session))
> goto free_session;
>
> + /* CTF writer trace stream setup */
> + if (ctf_writer__setup_stream(cw))
> + goto free_session;
> +
> /* CTF events setup */
> if (setup_events(cw, session))
> goto free_session;
> --
> 2.3.3.220.g9ab698f
>
On 2015/4/9 1:59, Jiri Olsa wrote:
> On Wed, Apr 08, 2015 at 12:49:20PM +0800, He Kuang wrote:
>> Due to babeltrace commit:
>> 7f800dc7c2a1 ("ir: make trace environment use bt_object")
>>
>> The trace->frozen flag is set in bt_ctf_trace_create_stream(), this flag
>> is checked before adding environment field to trace, and causes
>> ctf_writer__setup_env() failed. Fix this by setting all environment
>> fields before bt_ctf_trace_create_stream().
>>
>> Before this patch:
>> $ perf data convert --to-ctf=ctf
>> Error during CTF convert setup.
> have you tested with the latest babeltrace sources?
> this reminds me the bug they fixed recently, CCing Jeremie
>
> thanks,
> jirka
Yes, the latest babeltrace commit id:
dfdad2587b12d454e7235e01508a266d83e3e264
>> After this patch:
>> $ perf data convert --to-ctf=ctf
>> [ perf data convert: Converted 'perf.data' into CTF data 'ctf' ]
>> [ perf data convert: Converted and wrote 0.023 MB (596 samples) ]
>>
>> Signed-off-by: He Kuang <[email protected]>
>> ---
>> tools/perf/util/data-convert-bt.c | 30 ++++++++++++++++++++----------
>> 1 file changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
>> index a5b89b9..718dc8a 100644
>> --- a/tools/perf/util/data-convert-bt.c
>> +++ b/tools/perf/util/data-convert-bt.c
>> @@ -604,6 +604,22 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
>> return 0;
>> }
>>
>> +static int ctf_writer__setup_stream(struct ctf_writer *cw)
>> +{
>> + struct bt_ctf_stream *stream;
>> +
>> + /* CTF stream instance */
>> + stream = bt_ctf_writer_create_stream(cw->writer, cw->stream_class);
>> + if (!stream) {
>> + pr("Failed to create CTF stream.\n");
>> + return -1;
>> + }
>> +
>> + cw->stream = stream;
>> +
>> + return 0;
>> +}
>> +
>> static int ctf_writer__setup_env(struct ctf_writer *cw,
>> struct perf_session *session)
>> {
>> @@ -725,7 +741,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
>> {
>> struct bt_ctf_writer *writer;
>> struct bt_ctf_stream_class *stream_class;
>> - struct bt_ctf_stream *stream;
>> struct bt_ctf_clock *clock;
>>
>> /* CTF writer */
>> @@ -767,15 +782,6 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
>> if (ctf_writer__init_data(cw))
>> goto err_cleanup;
>>
>> - /* CTF stream instance */
>> - stream = bt_ctf_writer_create_stream(writer, stream_class);
>> - if (!stream) {
>> - pr("Failed to create CTF stream.\n");
>> - goto err_cleanup;
>> - }
>> -
>> - cw->stream = stream;
>> -
>> /* CTF clock writer setup */
>> if (bt_ctf_writer_add_clock(writer, clock)) {
>> pr("Failed to assign CTF clock to writer.\n");
>> @@ -830,6 +836,10 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
>> if (ctf_writer__setup_env(cw, session))
>> goto free_session;
>>
>> + /* CTF writer trace stream setup */
>> + if (ctf_writer__setup_stream(cw))
>> + goto free_session;
>> +
>> /* CTF events setup */
>> if (setup_events(cw, session))
>> goto free_session;
>> --
>> 2.3.3.220.g9ab698f
>>
>
Show message when errors occurred during conversion setup and conversion
process.
Before this patch:
$ ./perf data convert --to-ctf=ctf
$ echo $?
255
After this patch:
$ ./perf data convert --to-ctf=ctf
Error during conversion setup.
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/data-convert-bt.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index 7a12047..de80ded 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -896,6 +896,8 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
err = perf_session__process_events(session);
if (!err)
err = bt_ctf_stream_flush(cw->stream);
+ else
+ pr_err("Error during conversion.\n");
fprintf(stderr,
"[ perf data convert: Converted '%s' into CTF data '%s' ]\n",
@@ -906,11 +908,15 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
(double) c.events_size / 1024.0 / 1024.0,
c.events_count);
- /* its all good */
-free_session:
perf_session__delete(session);
+ ctf_writer__cleanup(cw);
+ return err;
+
+free_session:
+ perf_session__delete(session);
free_writer:
ctf_writer__cleanup(cw);
+ pr_err("Error during conversion setup.\n");
return err;
}
--
2.3.3.220.g9ab698f
Hi, jirka
On 2015/4/9 1:45, Jiri Olsa wrote:
> On Wed, Apr 08, 2015 at 12:49:19PM +0800, He Kuang wrote:
>> Show message when errors occurred during ctf conversion setup.
>>
>> Before this patch:
>> $ ./perf data convert --to-ctf=ctf
>> $ echo $?
>> 255
>>
>> After this patch:
>> $ ./perf data convert --to-ctf=ctf
>> Error during CTF convert setup.
> so I have like 5 more patches from the original CTF set
> which I'm holding until all works with tracecompass:
> http://marc.info/?l=linux-kernel&m=142736197610573&w=2
>
> Is it working for you? How do you test resulted CTF data?
>
> anyway the patch looks ok, just small nit below
I tested by using babeltrace binary and it works.
After receiving your reply, I test on the latest tracecompass. A
folder named 'ctf' is showed instead of the expected file
'ctf-data', this folder only contains the raw metadata and
perf-stream files but not analysed.
>> Signed-off-by: He Kuang <[email protected]>
>> ---
>> tools/perf/util/data-convert-bt.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
>> index dd17c9a..a5b89b9 100644
>> --- a/tools/perf/util/data-convert-bt.c
>> +++ b/tools/perf/util/data-convert-bt.c
>> @@ -847,11 +847,15 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
>> (double) c.events_size / 1024.0 / 1024.0,
>> c.events_count);
>>
>> - /* its all good */
>> -free_session:
>> perf_session__delete(session);
>> + ctf_writer__cleanup(cw);
>> +
> this leg can also fail due to:
>
> err = perf_session__process_events(session);
> if (!err)
> err = bt_ctf_stream_flush(cw->stream);
>
>
> so we might want to inform about that like:
> if (err)
> pr_err("Error during conversion.\n");
>
>
> thanks,
> jirka
>
>> + return err;
>>
>> +free_session:
>> + perf_session__delete(session);
>> free_writer:
>> ctf_writer__cleanup(cw);
>> + pr_err("Error during CTF convert setup.\n");
>> return err;
>> }
>> --
>> 2.3.3.220.g9ab698f
>
On Thu, Apr 09, 2015 at 03:56:00PM +0800, He Kuang wrote:
> Show message when errors occurred during conversion setup and conversion
> process.
>
> Before this patch:
> $ ./perf data convert --to-ctf=ctf
> $ echo $?
> 255
>
> After this patch:
> $ ./perf data convert --to-ctf=ctf
> Error during conversion setup.
>
> Signed-off-by: He Kuang <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
thanks,
jirka
> ---
> tools/perf/util/data-convert-bt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index 7a12047..de80ded 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -896,6 +896,8 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
> err = perf_session__process_events(session);
> if (!err)
> err = bt_ctf_stream_flush(cw->stream);
> + else
> + pr_err("Error during conversion.\n");
>
> fprintf(stderr,
> "[ perf data convert: Converted '%s' into CTF data '%s' ]\n",
> @@ -906,11 +908,15 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
> (double) c.events_size / 1024.0 / 1024.0,
> c.events_count);
>
> - /* its all good */
> -free_session:
> perf_session__delete(session);
> + ctf_writer__cleanup(cw);
>
> + return err;
> +
> +free_session:
> + perf_session__delete(session);
> free_writer:
> ctf_writer__cleanup(cw);
> + pr_err("Error during conversion setup.\n");
> return err;
> }
> --
> 2.3.3.220.g9ab698f
>
On Thu, Apr 09, 2015 at 04:19:20PM +0800, He Kuang wrote:
> Hi, jirka
> On 2015/4/9 1:45, Jiri Olsa wrote:
> >On Wed, Apr 08, 2015 at 12:49:19PM +0800, He Kuang wrote:
> >>Show message when errors occurred during ctf conversion setup.
> >>
> >>Before this patch:
> >> $ ./perf data convert --to-ctf=ctf
> >> $ echo $?
> >> 255
> >>
> >>After this patch:
> >> $ ./perf data convert --to-ctf=ctf
> >> Error during CTF convert setup.
> >so I have like 5 more patches from the original CTF set
> >which I'm holding until all works with tracecompass:
> > http://marc.info/?l=linux-kernel&m=142736197610573&w=2
> >
> >Is it working for you? How do you test resulted CTF data?
> >
> >anyway the patch looks ok, just small nit below
>
> I tested by using babeltrace binary and it works.
>
> After receiving your reply, I test on the latest tracecompass. A
> folder named 'ctf' is showed instead of the expected file
> 'ctf-data', this folder only contains the raw metadata and
> perf-stream files but not analysed.
CC-ing Alexandre from tracecompass devel ^^^
jirka
On 2015-04-09 05:46 AM, Jiri Olsa wrote:
> On Thu, Apr 09, 2015 at 04:19:20PM +0800, He Kuang wrote:
>> Hi, jirka
>> On 2015/4/9 1:45, Jiri Olsa wrote:
>>> On Wed, Apr 08, 2015 at 12:49:19PM +0800, He Kuang wrote:
>>>> Show message when errors occurred during ctf conversion setup.
>>>>
>>>> Before this patch:
>>>> $ ./perf data convert --to-ctf=ctf
>>>> $ echo $?
>>>> 255
>>>>
>>>> After this patch:
>>>> $ ./perf data convert --to-ctf=ctf
>>>> Error during CTF convert setup.
>>> so I have like 5 more patches from the original CTF set
>>> which I'm holding until all works with tracecompass:
>>> http://marc.info/?l=linux-kernel&m=142736197610573&w=2
>>>
>>> Is it working for you? How do you test resulted CTF data?
>>>
>>> anyway the patch looks ok, just small nit below
>> I tested by using babeltrace binary and it works.
>>
>> After receiving your reply, I test on the latest tracecompass. A
>> folder named 'ctf' is showed instead of the expected file
>> 'ctf-data', this folder only contains the raw metadata and
>> perf-stream files but not analysed.
> CC-ing Alexandre from tracecompass devel ^^^
Hi,
I just came back from vacation, sorry for not replying earlier!
I managed to compile perf with CTF support, but by using Babeltrace's
commit 5584a48. It fails to compile against current master, because of
private headers getting exposed. I reported that to the BT maintainers.
Then it seems there's another bug with Trace Compass's current master,
trace validation cannot fail, and any file will get imported with no
errors. We will look into this.
But the root of the problem was that the converted CTF trace was not
being recognized as valid. This is because some events define "stream_id
= 0;", and others don't specify a stream_id at all. It seems quite
random, see the full metadata here: http://pastebin.com/pACgV5JU
Is there a reason why some events specify a stream_id and some don't?
We could patch Trace Compass to accept it, since Babeltrace does. But
it's not very clear according to the spec, I'll check with the CTF guys
if it should be considered valid or not.
Cheers,
Alexandre
>
> jirka
Hi He,
This commit should fix the problem:
commit a0d129162d2fdd1a99553a6cfbdf4e77ad3f7334
Author: Jérémie Galarneau <[email protected]>
Date: Thu Apr 9 14:57:44 2015 -0400
Fix: Allow the addition of environment fields to a frozen trace
Commit 7f800dc7 introduced a behavior change which made it
impossible to add environment fields to a frozen trace (after the
creation of a stream).
This fix makes it possible to add new fields to a trace's
environment while making it impossible to modify existing fields
hereby restoring CTF Writer's v1.2 behavior.
Signed-off-by: Jérémie Galarneau <[email protected]>
Can you reproduce the problem with the latest Babeltrace master?
Otherwise, is there a branch I can checkout to try it out?
Jérémie
On Thu, Apr 9, 2015 at 3:38 AM, He Kuang <[email protected]> wrote:
> On 2015/4/9 1:59, Jiri Olsa wrote:
>>
>> On Wed, Apr 08, 2015 at 12:49:20PM +0800, He Kuang wrote:
>>>
>>> Due to babeltrace commit:
>>> 7f800dc7c2a1 ("ir: make trace environment use bt_object")
>>>
>>> The trace->frozen flag is set in bt_ctf_trace_create_stream(), this flag
>>> is checked before adding environment field to trace, and causes
>>> ctf_writer__setup_env() failed. Fix this by setting all environment
>>> fields before bt_ctf_trace_create_stream().
>>>
>>> Before this patch:
>>> $ perf data convert --to-ctf=ctf
>>> Error during CTF convert setup.
>>
>> have you tested with the latest babeltrace sources?
>> this reminds me the bug they fixed recently, CCing Jeremie
>>
>> thanks,
>> jirka
>
>
> Yes, the latest babeltrace commit id:
> dfdad2587b12d454e7235e01508a266d83e3e264
>
>>> After this patch:
>>> $ perf data convert --to-ctf=ctf
>>> [ perf data convert: Converted 'perf.data' into CTF data 'ctf' ]
>>> [ perf data convert: Converted and wrote 0.023 MB (596 samples) ]
>>>
>>> Signed-off-by: He Kuang <[email protected]>
>>> ---
>>> tools/perf/util/data-convert-bt.c | 30 ++++++++++++++++++++----------
>>> 1 file changed, 20 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/perf/util/data-convert-bt.c
>>> b/tools/perf/util/data-convert-bt.c
>>> index a5b89b9..718dc8a 100644
>>> --- a/tools/perf/util/data-convert-bt.c
>>> +++ b/tools/perf/util/data-convert-bt.c
>>> @@ -604,6 +604,22 @@ static int setup_events(struct ctf_writer *cw,
>>> struct perf_session *session)
>>> return 0;
>>> }
>>> +static int ctf_writer__setup_stream(struct ctf_writer *cw)
>>> +{
>>> + struct bt_ctf_stream *stream;
>>> +
>>> + /* CTF stream instance */
>>> + stream = bt_ctf_writer_create_stream(cw->writer,
>>> cw->stream_class);
>>> + if (!stream) {
>>> + pr("Failed to create CTF stream.\n");
>>> + return -1;
>>> + }
>>> +
>>> + cw->stream = stream;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int ctf_writer__setup_env(struct ctf_writer *cw,
>>> struct perf_session *session)
>>> {
>>> @@ -725,7 +741,6 @@ static int ctf_writer__init(struct ctf_writer *cw,
>>> const char *path)
>>> {
>>> struct bt_ctf_writer *writer;
>>> struct bt_ctf_stream_class *stream_class;
>>> - struct bt_ctf_stream *stream;
>>> struct bt_ctf_clock *clock;
>>> /* CTF writer */
>>> @@ -767,15 +782,6 @@ static int ctf_writer__init(struct ctf_writer *cw,
>>> const char *path)
>>> if (ctf_writer__init_data(cw))
>>> goto err_cleanup;
>>> - /* CTF stream instance */
>>> - stream = bt_ctf_writer_create_stream(writer, stream_class);
>>> - if (!stream) {
>>> - pr("Failed to create CTF stream.\n");
>>> - goto err_cleanup;
>>> - }
>>> -
>>> - cw->stream = stream;
>>> -
>>> /* CTF clock writer setup */
>>> if (bt_ctf_writer_add_clock(writer, clock)) {
>>> pr("Failed to assign CTF clock to writer.\n");
>>> @@ -830,6 +836,10 @@ int bt_convert__perf2ctf(const char *input, const
>>> char *path, bool force)
>>> if (ctf_writer__setup_env(cw, session))
>>> goto free_session;
>>> + /* CTF writer trace stream setup */
>>> + if (ctf_writer__setup_stream(cw))
>>> + goto free_session;
>>> +
>>> /* CTF events setup */
>>> if (setup_events(cw, session))
>>> goto free_session;
>>> --
>>> 2.3.3.220.g9ab698f
>>>
>>
>
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
Hi, Jérémie
On 2015/4/10 3:57, Jérémie Galarneau wrote:
> Hi He,
>
> This commit should fix the problem:
>
> commit a0d129162d2fdd1a99553a6cfbdf4e77ad3f7334
> Author: Jérémie Galarneau <[email protected]>
> Date: Thu Apr 9 14:57:44 2015 -0400
>
> Fix: Allow the addition of environment fields to a frozen trace
>
> Commit 7f800dc7 introduced a behavior change which made it
> impossible to add environment fields to a frozen trace (after the
> creation of a stream).
>
> This fix makes it possible to add new fields to a trace's
> environment while making it impossible to modify existing fields
> hereby restoring CTF Writer's v1.2 behavior.
>
> Signed-off-by: Jérémie Galarneau <[email protected]>
>
> Can you reproduce the problem with the latest Babeltrace master?
> Otherwise, is there a branch I can checkout to try it out?
>
> Jérémie
By updating to the latest libbabeltrace which contains commit
a0d12916, perf ctf conversion works with or without my patch.
>
> On Thu, Apr 9, 2015 at 3:38 AM, He Kuang <[email protected]> wrote:
>> On 2015/4/9 1:59, Jiri Olsa wrote:
>>> On Wed, Apr 08, 2015 at 12:49:20PM +0800, He Kuang wrote:
>>>> Due to babeltrace commit:
>>>> 7f800dc7c2a1 ("ir: make trace environment use bt_object")
>>>>
>>>> The trace->frozen flag is set in bt_ctf_trace_create_stream(), this flag
>>>> is checked before adding environment field to trace, and causes
>>>> ctf_writer__setup_env() failed. Fix this by setting all environment
>>>> fields before bt_ctf_trace_create_stream().
>>>>
>>>> Before this patch:
>>>> $ perf data convert --to-ctf=ctf
>>>> Error during CTF convert setup.
>>> have you tested with the latest babeltrace sources?
>>> this reminds me the bug they fixed recently, CCing Jeremie
>>>
>>> thanks,
>>> jirka
>>
>> Yes, the latest babeltrace commit id:
>> dfdad2587b12d454e7235e01508a266d83e3e264
>>
>>>> After this patch:
>>>> $ perf data convert --to-ctf=ctf
>>>> [ perf data convert: Converted 'perf.data' into CTF data 'ctf' ]
>>>> [ perf data convert: Converted and wrote 0.023 MB (596 samples) ]
>>>>
>>>> Signed-off-by: He Kuang <[email protected]>
>>>> ---
>>>> tools/perf/util/data-convert-bt.c | 30 ++++++++++++++++++++----------
>>>> 1 file changed, 20 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/data-convert-bt.c
>>>> b/tools/perf/util/data-convert-bt.c
>>>> index a5b89b9..718dc8a 100644
>>>> --- a/tools/perf/util/data-convert-bt.c
>>>> +++ b/tools/perf/util/data-convert-bt.c
>>>> @@ -604,6 +604,22 @@ static int setup_events(struct ctf_writer *cw,
>>>> struct perf_session *session)
>>>> return 0;
>>>> }
>>>> +static int ctf_writer__setup_stream(struct ctf_writer *cw)
>>>> +{
>>>> + struct bt_ctf_stream *stream;
>>>> +
>>>> + /* CTF stream instance */
>>>> + stream = bt_ctf_writer_create_stream(cw->writer,
>>>> cw->stream_class);
>>>> + if (!stream) {
>>>> + pr("Failed to create CTF stream.\n");
>>>> + return -1;
>>>> + }
>>>> +
>>>> + cw->stream = stream;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int ctf_writer__setup_env(struct ctf_writer *cw,
>>>> struct perf_session *session)
>>>> {
>>>> @@ -725,7 +741,6 @@ static int ctf_writer__init(struct ctf_writer *cw,
>>>> const char *path)
>>>> {
>>>> struct bt_ctf_writer *writer;
>>>> struct bt_ctf_stream_class *stream_class;
>>>> - struct bt_ctf_stream *stream;
>>>> struct bt_ctf_clock *clock;
>>>> /* CTF writer */
>>>> @@ -767,15 +782,6 @@ static int ctf_writer__init(struct ctf_writer *cw,
>>>> const char *path)
>>>> if (ctf_writer__init_data(cw))
>>>> goto err_cleanup;
>>>> - /* CTF stream instance */
>>>> - stream = bt_ctf_writer_create_stream(writer, stream_class);
>>>> - if (!stream) {
>>>> - pr("Failed to create CTF stream.\n");
>>>> - goto err_cleanup;
>>>> - }
>>>> -
>>>> - cw->stream = stream;
>>>> -
>>>> /* CTF clock writer setup */
>>>> if (bt_ctf_writer_add_clock(writer, clock)) {
>>>> pr("Failed to assign CTF clock to writer.\n");
>>>> @@ -830,6 +836,10 @@ int bt_convert__perf2ctf(const char *input, const
>>>> char *path, bool force)
>>>> if (ctf_writer__setup_env(cw, session))
>>>> goto free_session;
>>>> + /* CTF writer trace stream setup */
>>>> + if (ctf_writer__setup_stream(cw))
>>>> + goto free_session;
>>>> +
>>>> /* CTF events setup */
>>>> if (setup_events(cw, session))
>>>> goto free_session;
>>>> --
>>>> 2.3.3.220.g9ab698f
>>>>
>>
>
>
On Thu, Apr 09, 2015 at 10:37:47AM -0400, Alexandre Montplaisir wrote:
> On 2015-04-09 05:46 AM, Jiri Olsa wrote:
> >On Thu, Apr 09, 2015 at 04:19:20PM +0800, He Kuang wrote:
> >>Hi, jirka
> >>On 2015/4/9 1:45, Jiri Olsa wrote:
> >>>On Wed, Apr 08, 2015 at 12:49:19PM +0800, He Kuang wrote:
> >>>>Show message when errors occurred during ctf conversion setup.
> >>>>
> >>>>Before this patch:
> >>>> $ ./perf data convert --to-ctf=ctf
> >>>> $ echo $?
> >>>> 255
> >>>>
> >>>>After this patch:
> >>>> $ ./perf data convert --to-ctf=ctf
> >>>> Error during CTF convert setup.
> >>>so I have like 5 more patches from the original CTF set
> >>>which I'm holding until all works with tracecompass:
> >>> http://marc.info/?l=linux-kernel&m=142736197610573&w=2
> >>>
> >>>Is it working for you? How do you test resulted CTF data?
> >>>
> >>>anyway the patch looks ok, just small nit below
> >>I tested by using babeltrace binary and it works.
> >>
> >>After receiving your reply, I test on the latest tracecompass. A
> >>folder named 'ctf' is showed instead of the expected file
> >>'ctf-data', this folder only contains the raw metadata and
> >>perf-stream files but not analysed.
> >CC-ing Alexandre from tracecompass devel ^^^
>
> Hi,
>
> I just came back from vacation, sorry for not replying earlier!
>
> I managed to compile perf with CTF support, but by using Babeltrace's commit
> 5584a48. It fails to compile against current master, because of private
> headers getting exposed. I reported that to the BT maintainers.
there's fix in babeltrace tree already
>
> Then it seems there's another bug with Trace Compass's current master, trace
> validation cannot fail, and any file will get imported with no errors. We
> will look into this.
> But the root of the problem was that the converted CTF trace was not being
> recognized as valid. This is because some events define "stream_id = 0;",
> and others don't specify a stream_id at all. It seems quite random, see the
> full metadata here: http://pastebin.com/pACgV5JU
>
> Is there a reason why some events specify a stream_id and some don't?
hum, that seems like a bug.. I'll check
>
> We could patch Trace Compass to accept it, since Babeltrace does. But it's
> not very clear according to the spec, I'll check with the CTF guys if it
> should be considered valid or not.
thanks,
jirka
On Fri, Apr 10, 2015 at 02:05:45PM +0200, Jiri Olsa wrote:
SNIP
> > >>I tested by using babeltrace binary and it works.
> > >>
> > >>After receiving your reply, I test on the latest tracecompass. A
> > >>folder named 'ctf' is showed instead of the expected file
> > >>'ctf-data', this folder only contains the raw metadata and
> > >>perf-stream files but not analysed.
> > >CC-ing Alexandre from tracecompass devel ^^^
> >
> > Hi,
> >
> > I just came back from vacation, sorry for not replying earlier!
> >
> > I managed to compile perf with CTF support, but by using Babeltrace's commit
> > 5584a48. It fails to compile against current master, because of private
> > headers getting exposed. I reported that to the BT maintainers.
>
> there's fix in babeltrace tree already
>
> >
> > Then it seems there's another bug with Trace Compass's current master, trace
> > validation cannot fail, and any file will get imported with no errors. We
> > will look into this.
> > But the root of the problem was that the converted CTF trace was not being
> > recognized as valid. This is because some events define "stream_id = 0;",
> > and others don't specify a stream_id at all. It seems quite random, see the
> > full metadata here: http://pastebin.com/pACgV5JU
> >
> > Is there a reason why some events specify a stream_id and some don't?
>
> hum, that seems like a bug.. I'll check
>
ok, found the problem.. the "stream_id" event_class's attribute is created
only when the instance of the event (not event_class) is created
so you'll see the stream_id attribute only for events, that
we actually got data for.. the rest is without, because
their instance was never created
seems to me like libbabeltrace bug, unless the application should
take care about stream_id attribute.. but it's not the case for
the rest of the 'internal' attributes.. Jeremie?
anyway, I made a attached workaround and it all works nicely again,
tracecompass is happy and shows the data properly
I put all the pending ctf changes (plus the workaround) into:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/ctf branch
jirka
---
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index 977cc3f98d8f..d7f03dcb1700 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -803,6 +803,7 @@ static int add_generic_types(struct ctf_writer *cw, struct perf_evsel *evsel,
static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
{
struct bt_ctf_event_class *event_class;
+ struct bt_ctf_event *event;
struct evsel_priv *priv;
const char *name = perf_evsel__name(evsel);
int ret;
@@ -833,6 +834,14 @@ static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
if (!priv)
goto err;
+ event = bt_ctf_event_create(event_class);
+ if (!event) {
+ pr_err("Failed to create an CTF event\n");
+ goto err;
+ }
+
+ bt_ctf_event_put(event);
+
priv->event_class = event_class;
evsel->priv = priv;
return 0;
On Fri, Apr 10, 2015 at 03:39:25PM +0800, He Kuang wrote:
> Hi, J?r?mie
> On 2015/4/10 3:57, J?r?mie Galarneau wrote:
> >Hi He,
> >
> >This commit should fix the problem:
> >
> >commit a0d129162d2fdd1a99553a6cfbdf4e77ad3f7334
> >Author: J?r?mie Galarneau <[email protected]>
> >Date: Thu Apr 9 14:57:44 2015 -0400
> >
> > Fix: Allow the addition of environment fields to a frozen trace
> >
> > Commit 7f800dc7 introduced a behavior change which made it
> > impossible to add environment fields to a frozen trace (after the
> > creation of a stream).
> >
> > This fix makes it possible to add new fields to a trace's
> > environment while making it impossible to modify existing fields
> > hereby restoring CTF Writer's v1.2 behavior.
> >
> > Signed-off-by: J?r?mie Galarneau <[email protected]>
> >
> >Can you reproduce the problem with the latest Babeltrace master?
> >Otherwise, is there a branch I can checkout to try it out?
> >
> >J?r?mie
>
> By updating to the latest libbabeltrace which contains commit
> a0d12916, perf ctf conversion works with or without my patch.
works nicely for me too, thanks!
jirka
On Fri, Apr 10, 2015 at 8:37 AM, Jiri Olsa <[email protected]> wrote:
> On Fri, Apr 10, 2015 at 02:05:45PM +0200, Jiri Olsa wrote:
>
> SNIP
>
>> > >>I tested by using babeltrace binary and it works.
>> > >>
>> > >>After receiving your reply, I test on the latest tracecompass. A
>> > >>folder named 'ctf' is showed instead of the expected file
>> > >>'ctf-data', this folder only contains the raw metadata and
>> > >>perf-stream files but not analysed.
>> > >CC-ing Alexandre from tracecompass devel ^^^
>> >
>> > Hi,
>> >
>> > I just came back from vacation, sorry for not replying earlier!
>> >
>> > I managed to compile perf with CTF support, but by using Babeltrace's commit
>> > 5584a48. It fails to compile against current master, because of private
>> > headers getting exposed. I reported that to the BT maintainers.
>>
>> there's fix in babeltrace tree already
>>
>> >
>> > Then it seems there's another bug with Trace Compass's current master, trace
>> > validation cannot fail, and any file will get imported with no errors. We
>> > will look into this.
>> > But the root of the problem was that the converted CTF trace was not being
>> > recognized as valid. This is because some events define "stream_id = 0;",
>> > and others don't specify a stream_id at all. It seems quite random, see the
>> > full metadata here: http://pastebin.com/pACgV5JU
>> >
>> > Is there a reason why some events specify a stream_id and some don't?
>>
>> hum, that seems like a bug.. I'll check
>>
>
> ok, found the problem.. the "stream_id" event_class's attribute is created
> only when the instance of the event (not event_class) is created
>
> so you'll see the stream_id attribute only for events, that
> we actually got data for.. the rest is without, because
> their instance was never created
>
> seems to me like libbabeltrace bug, unless the application should
> take care about stream_id attribute.. but it's not the case for
> the rest of the 'internal' attributes.. Jeremie?
According to the spec, the stream_id attribute can be left unspecified
if only one stream is defined. However, is seems Babeltrace's CTF-Writer
will leave the stream_id out of the event's declaration even when
multiple streams are defined.
I'll submit a fix.
Thanks.
Jérémie
>
> anyway, I made a attached workaround and it all works nicely again,
> tracecompass is happy and shows the data properly
>
> I put all the pending ctf changes (plus the workaround) into:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> perf/ctf branch
>
> jirka
>
>
> ---
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index 977cc3f98d8f..d7f03dcb1700 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -803,6 +803,7 @@ static int add_generic_types(struct ctf_writer *cw, struct perf_evsel *evsel,
> static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
> {
> struct bt_ctf_event_class *event_class;
> + struct bt_ctf_event *event;
> struct evsel_priv *priv;
> const char *name = perf_evsel__name(evsel);
> int ret;
> @@ -833,6 +834,14 @@ static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
> if (!priv)
> goto err;
>
> + event = bt_ctf_event_create(event_class);
> + if (!event) {
> + pr_err("Failed to create an CTF event\n");
> + goto err;
> + }
> +
> + bt_ctf_event_put(event);
> +
> priv->event_class = event_class;
> evsel->priv = priv;
> return 0;
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
Should be fixed as of this commit.
commit 29664b2a3a15c7233d916887d2f58fc42e18521e
Author: Philippe Proulx <[email protected]>
Date: Mon Apr 13 18:14:59 2015 -0400
Fix: ir: make sure "stream_id" attr is always right
Make sure the "stream_id" attribute of all the event
classes of a given stream class is updated at the following
places:
* user sets the stream class ID manually: calling
bt_ctf_stream_class_set_id()
* stream class ID is automatically set: in
bt_ctf_trace_add_stream_class()
* an event class is added to an existing stream
class: in bt_ctf_stream_class_add_event_class()
Signed-off-by: Philippe Proulx <[email protected]>
Signed-off-by: Jérémie Galarneau <[email protected]>
Jérémie
On Mon, Apr 13, 2015 at 4:30 PM, Jérémie Galarneau
<[email protected]> wrote:
> On Fri, Apr 10, 2015 at 8:37 AM, Jiri Olsa <[email protected]> wrote:
>> On Fri, Apr 10, 2015 at 02:05:45PM +0200, Jiri Olsa wrote:
>>
>> SNIP
>>
>>> > >>I tested by using babeltrace binary and it works.
>>> > >>
>>> > >>After receiving your reply, I test on the latest tracecompass. A
>>> > >>folder named 'ctf' is showed instead of the expected file
>>> > >>'ctf-data', this folder only contains the raw metadata and
>>> > >>perf-stream files but not analysed.
>>> > >CC-ing Alexandre from tracecompass devel ^^^
>>> >
>>> > Hi,
>>> >
>>> > I just came back from vacation, sorry for not replying earlier!
>>> >
>>> > I managed to compile perf with CTF support, but by using Babeltrace's commit
>>> > 5584a48. It fails to compile against current master, because of private
>>> > headers getting exposed. I reported that to the BT maintainers.
>>>
>>> there's fix in babeltrace tree already
>>>
>>> >
>>> > Then it seems there's another bug with Trace Compass's current master, trace
>>> > validation cannot fail, and any file will get imported with no errors. We
>>> > will look into this.
>>> > But the root of the problem was that the converted CTF trace was not being
>>> > recognized as valid. This is because some events define "stream_id = 0;",
>>> > and others don't specify a stream_id at all. It seems quite random, see the
>>> > full metadata here: http://pastebin.com/pACgV5JU
>>> >
>>> > Is there a reason why some events specify a stream_id and some don't?
>>>
>>> hum, that seems like a bug.. I'll check
>>>
>>
>> ok, found the problem.. the "stream_id" event_class's attribute is created
>> only when the instance of the event (not event_class) is created
>>
>> so you'll see the stream_id attribute only for events, that
>> we actually got data for.. the rest is without, because
>> their instance was never created
>>
>> seems to me like libbabeltrace bug, unless the application should
>> take care about stream_id attribute.. but it's not the case for
>> the rest of the 'internal' attributes.. Jeremie?
>
> According to the spec, the stream_id attribute can be left unspecified
> if only one stream is defined. However, is seems Babeltrace's CTF-Writer
> will leave the stream_id out of the event's declaration even when
> multiple streams are defined.
>
> I'll submit a fix.
>
> Thanks.
> Jérémie
>
>>
>> anyway, I made a attached workaround and it all works nicely again,
>> tracecompass is happy and shows the data properly
>>
>> I put all the pending ctf changes (plus the workaround) into:
>> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>> perf/ctf branch
>>
>> jirka
>>
>>
>> ---
>> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
>> index 977cc3f98d8f..d7f03dcb1700 100644
>> --- a/tools/perf/util/data-convert-bt.c
>> +++ b/tools/perf/util/data-convert-bt.c
>> @@ -803,6 +803,7 @@ static int add_generic_types(struct ctf_writer *cw, struct perf_evsel *evsel,
>> static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
>> {
>> struct bt_ctf_event_class *event_class;
>> + struct bt_ctf_event *event;
>> struct evsel_priv *priv;
>> const char *name = perf_evsel__name(evsel);
>> int ret;
>> @@ -833,6 +834,14 @@ static int add_event(struct ctf_writer *cw, struct perf_evsel *evsel)
>> if (!priv)
>> goto err;
>>
>> + event = bt_ctf_event_create(event_class);
>> + if (!event) {
>> + pr_err("Failed to create an CTF event\n");
>> + goto err;
>> + }
>> +
>> + bt_ctf_event_put(event);
>> +
>> priv->event_class = event_class;
>> evsel->priv = priv;
>> return 0;
>
>
>
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
On Tue, Apr 14, 2015 at 01:47:51PM -0400, J?r?mie Galarneau wrote:
> Should be fixed as of this commit.
>
> commit 29664b2a3a15c7233d916887d2f58fc42e18521e
> Author: Philippe Proulx <[email protected]>
> Date: Mon Apr 13 18:14:59 2015 -0400
>
> Fix: ir: make sure "stream_id" attr is always right
>
> Make sure the "stream_id" attribute of all the event
> classes of a given stream class is updated at the following
> places:
>
> * user sets the stream class ID manually: calling
> bt_ctf_stream_class_set_id()
> * stream class ID is automatically set: in
> bt_ctf_trace_add_stream_class()
> * an event class is added to an existing stream
> class: in bt_ctf_stream_class_add_event_class()
>
> Signed-off-by: Philippe Proulx <[email protected]>
> Signed-off-by: J?r?mie Galarneau <[email protected]>
nice, works for me
thanks,
jirka
On Thu, Apr 09, 2015 at 11:45:21AM +0200, Jiri Olsa wrote:
> On Thu, Apr 09, 2015 at 03:56:00PM +0800, He Kuang wrote:
> > Show message when errors occurred during conversion setup and conversion
> > process.
> >
> > Before this patch:
> > $ ./perf data convert --to-ctf=ctf
> > $ echo $?
> > 255
> >
> > After this patch:
> > $ ./perf data convert --to-ctf=ctf
> > Error during conversion setup.
> >
> > Signed-off-by: He Kuang <[email protected]>
>
> Acked-by: Jiri Olsa <[email protected]>
I'll repost this one with my CTF changes queue,
the 2/2 patch is not needed now as it got fixed
in the libbabletrace
jirka