Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
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 > 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 > Signed-off-by: Jérémie Galarneau nice, works for me thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
Should be fixed as of this commit. commit 29664b2a3a15c7233d916887d2f58fc42e18521e Author: Philippe Proulx 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 Signed-off-by: Jérémie Galarneau Jérémie On Mon, Apr 13, 2015 at 4:30 PM, Jérémie Galarneau wrote: > On Fri, Apr 10, 2015 at 8:37 AM, Jiri Olsa 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
On Fri, Apr 10, 2015 at 8:37 AM, Jiri Olsa 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] perf data: Show error message when ctf setup failed
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/