Re: [PATCH 2/2] perf data: Fix ctf_writer setupenv failure

[He Kuang]

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 <jeremie.galarneau@xxxxxxxxxxxx>
> 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 <jeremie.galarneau@xxxxxxxxxxxx>
>
> 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 <hekuang@xxxxxxxxxx> 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 <hekuang@xxxxxxxxxx>
> > > > ---
> > > >    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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/