lkml.org 
[lkml]   [2016]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/54] perf data: Fix releasing event_class
    Em Fri, Feb 12, 2016 at 01:19:42PM +0100, Jiri Olsa escreveu:
    > On Thu, Feb 11, 2016 at 07:04:13PM -0300, Arnaldo Carvalho de Melo wrote:
    > > Em Fri, Feb 05, 2016 at 02:01:30PM +0000, Wang Nan escreveu:
    > > > A new patch of libbabeltrace [1] reveals a object leak problem in
    > > > perf data CTF support: perf code never release event_class which is
    > > > allocated in add_event() and stored in evsel's private field.
    > > >
    > > > If libbabeltrace has the above patch applied, leaking event_class
    > > > prevent writer being destroied and flushing metadata. For example:
    > >
    > > Ok, so if the user has an older version of this libbabeltrace, what
    > > happens?
    >
    > it's standard cleanup that should be there even for old version,
    >
    > IIUC the problem is that new version of libbabeltrace started
    > to check on refcounts on some exit function and gets crazy
    > if there's a mess/leak
    >
    > IIRC I've already acked this one

    Ok, Wang, if you notice such Acked-by, collect them, i.e. add them to
    new versions of your patchkits, that helps me in processing them, i.e.
    knowing that there was already discussion/acknowledgement from people
    directly involved in the affected code.

    thanks,

    - Arnaldo

    > jirka
    >
    > >
    > > Would he/she gets some warning about the requirement of a later version?
    > >
    > > - Arnaldo
    > >
    > > > $ ./perf record ls
    > > > Lowering default frequency rate to 500.
    > > > Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
    > > > perf.data
    > > > [ perf record: Woken up 1 times to write data ]
    > > > [ perf record: Captured and wrote 0.012 MB perf.data (12 samples) ]
    > > > $ ./perf data convert --to-ctf ./out.ctf
    > > > [ perf data convert: Converted 'perf.data' into CTF data './out.ctf' ]
    > > > [ perf data convert: Converted and wrote 0.000 MB (12 samples) ]
    > > > $ cat ./out.ctf/metadata
    > > > $ ls -l ./out.ctf/metadata
    > > > -rw-r----- 1 w00229757 mm 0 Jan 27 10:49 ./out.ctf/metadata
    > > >
    > > > The correct result should be:
    > > > ...
    > > > $ cat ./out.ctf/metadata
    > > > /* CTF 1.8 */
    > > >
    > > > trace {
    > > > [SNIP]
    > > >
    > > > $ ls -l ./out.ctf/metadata
    > > > -rw-r----- 1 w00229757 mm 2446 Jan 27 10:52 ./out.ctf/metadata
    > > >
    > > > The full story is:
    > > >
    > > > Patch [1] of babeltrace redesign reference counting scheme. In that
    > > > patch:
    > > >
    > > > * writer <- trace (bt_ctf_writer_create)
    > > > * trace <- stream_class (bt_ctf_trace_add_stream_class)
    > > > * stream_class <- event_class (bt_ctf_stream_class_add_event_class)
    > > > ('<-' means 'is a parent of')
    > > >
    > > > Holding of event_class causes reference count of corresponding
    > > > 'writer' increases through parent chain. Perf expect 'writer' is
    > > > released (so metadata is flushed) through bt_ctf_writer_put() in
    > > > ctf_writer__cleanup(). However, since it never release event_class,
    > > > the reference of 'writer' won't be reduced, so bt_ctf_writer_put()
    > > > won't lead releasing of writer.
    > > >
    > > > Before this CTF patch, !(writer <- trace). Even event_class leak,
    > > > writer is able to be released.
    > > >
    > > > [1] https://github.com/efficios/babeltrace/commit/e6a8e8e4744633807083a077ff9f101eb97d9801
    > > >
    > > > Signed-off-by: Wang Nan <wangnan0@huawei.com>
    > > > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
    > > > Cc: Jiri Olsa <jolsa@kernel.org>
    > > > Cc: Jérémie Galarneau <jeremie.galarneau@efficios.com>
    > > > Cc: Zefan Li <lizefan@huawei.com>
    > > > Cc: pi3orama@163.com
    > > > ---
    > > > tools/perf/util/data-convert-bt.c | 18 ++++++++++++++++++
    > > > 1 file changed, 18 insertions(+)
    > > >
    > > > diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
    > > > index 34cd1e4..b722e57 100644
    > > > --- a/tools/perf/util/data-convert-bt.c
    > > > +++ b/tools/perf/util/data-convert-bt.c
    > > > @@ -858,6 +858,23 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
    > > > return 0;
    > > > }
    > > >
    > > > +static void cleanup_events(struct perf_session *session)
    > > > +{
    > > > + struct perf_evlist *evlist = session->evlist;
    > > > + struct perf_evsel *evsel;
    > > > +
    > > > + evlist__for_each(evlist, evsel) {
    > > > + struct evsel_priv *priv;
    > > > +
    > > > + priv = evsel->priv;
    > > > + bt_ctf_event_class_put(priv->event_class);
    > > > + zfree(&evsel->priv);
    > > > + }
    > > > +
    > > > + perf_evlist__delete(evlist);
    > > > + session->evlist = NULL;
    > > > +}
    > > > +
    > > > static int setup_streams(struct ctf_writer *cw, struct perf_session *session)
    > > > {
    > > > struct ctf_stream **stream;
    > > > @@ -1171,6 +1188,7 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
    > > > (double) c.events_size / 1024.0 / 1024.0,
    > > > c.events_count);
    > > >
    > > > + cleanup_events(session);
    > > > perf_session__delete(session);
    > > > ctf_writer__cleanup(cw);
    > > >
    > > > --
    > > > 1.8.3.4

    \
     
     \ /
      Last update: 2016-02-12 17:01    [W:5.707 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site