linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jérémie Galarneau" <jeremie.galarneau@efficios.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Alexandre Montplaisir <alexmonthy@voxpopuli.im>,
	He Kuang <hekuang@huawei.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Jiri Olsa <jolsa@kernel.org>,
	acme@kernel.org, a.p.zijlstra@chello.nl, mingo@redhat.com,
	Wang Nan <wangnan0@huawei.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] perf data: Show error message when ctf setup failed
Date: Tue, 14 Apr 2015 13:47:51 -0400	[thread overview]
Message-ID: <CA+jJMxuTF4E=3FSC8ya5z9ZCgfRtKqGOXH7dqEH+jOWk+aFr4g@mail.gmail.com> (raw)
In-Reply-To: <CA+jJMxsKAXPVxZxdgt4Wu9kjXPjnZ7FOqkSxY7x+zg57z2z-=w@mail.gmail.com>

Should be fixed as of this commit.

commit 29664b2a3a15c7233d916887d2f58fc42e18521e
Author: Philippe Proulx <eeppeliteloop@gmail.com>
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 <eeppeliteloop@gmail.com>
    Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

Jérémie

On Mon, Apr 13, 2015 at 4:30 PM, Jérémie Galarneau
<jeremie.galarneau@efficios.com> wrote:
> On Fri, Apr 10, 2015 at 8:37 AM, Jiri Olsa <jolsa@redhat.com> 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

  reply	other threads:[~2015-04-14 17:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-08  4:49 [PATCH 1/2] perf data: Show error message when ctf setup failed He Kuang
2015-04-08  4:49 ` [PATCH 2/2] perf data: Fix ctf_writer setupenv failure He Kuang
2015-04-08 17:59   ` Jiri Olsa
2015-04-09  7:38     ` He Kuang
2015-04-09 19:57       ` Jérémie Galarneau
2015-04-10  7:39         ` He Kuang
2015-04-10 12:38           ` Jiri Olsa
2015-04-08 17:45 ` [PATCH 1/2] perf data: Show error message when ctf setup failed Jiri Olsa
2015-04-09  7:56   ` [PATCHv2 1/2] perf data: Show error message when conversion failed He Kuang
2015-04-09  9:45     ` Jiri Olsa
2015-04-18 14:00       ` Jiri Olsa
2015-04-09  8:19   ` [PATCH 1/2] perf data: Show error message when ctf setup failed He Kuang
2015-04-09  9:46     ` Jiri Olsa
2015-04-09 14:37       ` Alexandre Montplaisir
2015-04-10 12:05         ` Jiri Olsa
2015-04-10 12:37           ` Jiri Olsa
2015-04-13 20:30             ` Jérémie Galarneau
2015-04-14 17:47               ` Jérémie Galarneau [this message]
2015-04-18 13:58                 ` Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+jJMxuTF4E=3FSC8ya5z9ZCgfRtKqGOXH7dqEH+jOWk+aFr4g@mail.gmail.com' \
    --to=jeremie.galarneau@efficios.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexmonthy@voxpopuli.im \
    --cc=bigeasy@linutronix.de \
    --cc=hekuang@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).