[lttng-dev] [PATCH lttng-tools] Fix: Fail to add several perf_thread context
Jérémie Galarneau
jeremie.galarneau at efficios.com
Thu Nov 6 17:52:37 EST 2014
On Mon, Nov 3, 2014 at 4:30 PM, Alexander Grigoriev <alexgri at tbricks.com> wrote:
> Hi Jérémie,
>
> "lttng add-context" should not fail when several unique performance counters
> added after start program with tracepoints. Now "lttng add-context" fail
> when several unique perf_tread context added after start program with
> tracepoints.
>
Alright. Read on for comments about the fix.
> I wrote about this problem:
>
> ---------- Forwarded message ----------
> From: Alexander Grigoriev <alexgri at tbricks.com>
> Date: Tue, Sep 30, 2014 at 4:23 PM
> Subject: Problem with perf:thread in 2.5
> To: lttng-dev at lists.lttng.org
>
>
> Hello,
>
> I use lttng-2.5 and linux-3.8.13. When I try to add context with several
> performance counters after start program with tracepoints, I get the
> following error:
>
>> app_with_lttng & // { tracepoint(test, test, 1); sleep(1000); }
> [1] 19297
>
>> lttng destroy -a
> No session found, nothing to do.
>
>> lttng -vvv create test
> DEBUG3 - 14:54:42.713573 [19313/19313]: URI string:
> file:///home/alexgri/lttng-traces/test-20140930-145442 (in uri_parse() at
> uri.c:293)
> DEBUG3 - 14:54:42.713607 [19313/19313]: URI file destination:
> /home/alexgri/lttng-traces/test-20140930-145442 (in uri_parse() at
> uri.c:330)
> DEBUG3 - 14:54:42.713616 [19313/19313]: URI dtype: 3, proto: 0, host: ,
> subdir: , ctrl: 0, data: 0 (in uri_parse() at uri.c:507)
> DEBUG1 - 14:54:42.713667 [19313/19313]: LSM cmd type : 8 (in
> send_session_msg() at lttng-ctl.c:131)
> Session test created.
> Traces will be written in /home/alexgri/lttng-traces/test-20140930-145442
> DEBUG1 - 14:54:42.715819 [19313/19313]: Init config session in /home/alexgri
> (in config_init() at conf.c:295)
>
>> lttng -vvv add-context -u -t vpid -t perf:thread:cpu-cycles -t
>> perf:thread:instructions -t perf:thread:cache-misses -s test
> DEBUG1 - 14:54:52.124745 [19316/19316]: Adding context... (in add_context()
> at commands/add_context.c:600)
> DEBUG1 - 14:54:52.133393 [19316/19316]: LSM cmd type : 0 (in
> send_session_msg() at lttng-ctl.c:131)
> UST context vpid added to all channels
> DEBUG1 - 14:54:52.153445 [19316/19316]: Adding context... (in add_context()
> at commands/add_context.c:600)
> DEBUG1 - 14:54:52.153499 [19316/19316]: LSM cmd type : 0 (in
> send_session_msg() at lttng-ctl.c:131)
> UST context perf:thread:cpu-cycles added to all channels
> DEBUG1 - 14:54:52.154356 [19316/19316]: Adding context... (in add_context()
> at commands/add_context.c:600)
> DEBUG1 - 14:54:52.154392 [19316/19316]: LSM cmd type : 0 (in
> send_session_msg() at lttng-ctl.c:131)
> Error: perf:thread:instructions: UST context already exist
> DEBUG1 - 14:54:52.154469 [19316/19316]: Adding context... (in add_context()
> at commands/add_context.c:600)
> DEBUG1 - 14:54:52.154525 [19316/19316]: LSM cmd type : 0 (in
> send_session_msg() at lttng-ctl.c:131)
> Error: perf:thread:cache-misses: UST context already exist
> Warning: Some command(s) went wrong
> DEBUG1 - 14:54:52.154620 [19316/19316]: Clean exit (in clean_exit() at
> lttng.c:167)
>
> 'lttng-sessiond -vvv' logs in attachment
>
> Thanks,
> Alexander
> ---------------------------------------------------
>
>
> Thanks,
> Alexander
>
> On Mon, Nov 3, 2014 at 11:28 PM, Jérémie Galarneau
> <jeremie.galarneau at efficios.com> wrote:
>>
>> Hi Alexander,
>>
>> Just to make sure I understand the problem, is your point that "lttng
>> add-context" should not fail when multiple contexts are added just
>> because one of them was already present? If not, could you provide a
>> log demonstrating the issue?
>>
>> Thanks!
>> Jérémie
>>
>> On Sat, Nov 1, 2014 at 7:14 PM, Alexander Grigoriev <alexgri at tbricks.com>
>> wrote:
>> > In commit aa3514e96f12c13f681a81ea275dc51dd63473c8 not removed old check
>> > for duplicate context type in create_ust_app_channel_context() function.
>> > If
>> > the one of perf_tread contexts was already added, in the next
>> > similar operation function returns error code EEXIST "UST context
>> > already exist".
>> >
>> > Signed-off-by: Alexander Grigoriev <alexgri at tbricks.com>
>> > ---
>> > src/bin/lttng-sessiond/ust-app.c | 7 -------
>> > 1 file changed, 7 deletions(-)
>> >
>> > diff --git a/src/bin/lttng-sessiond/ust-app.c
>> > b/src/bin/lttng-sessiond/ust-app.c
>> > index f8e9693..7f1594b 100644
>> > --- a/src/bin/lttng-sessiond/ust-app.c
>> > +++ b/src/bin/lttng-sessiond/ust-app.c
>> > @@ -1940,13 +1940,6 @@ int create_ust_app_channel_context(struct
>> > ust_app_session *ua_sess,
>> >
>> > DBG2("UST app adding context to channel %s", ua_chan->name);
>> >
>> > - lttng_ht_lookup(ua_chan->ctx, (void *)((unsigned
>> > long)uctx->ctx), &iter);
>> > - node = lttng_ht_iter_get_node_ulong(&iter);
>> > - if (node != NULL) {
>> > - ret = -EEXIST;
>> > - goto error;
>> > - }
>> > -
The fix is a bit more complex than removing this check since we do
want to return an error when a user adds a context which has already
been enabled in a given channel.
Basically, that part of the code is fine as is, but the matching
function must be changed to account for both the context type and the
perf thread counter name when the context type is
LTTNG_UST_CONTEXT_PERF_THREAD_COUNTER.
I have implemented a fix in master as:
commit 6a6b20683ccf29e16baceef5d29357001c2c713a
Author: Jérémie Galarneau <jeremie.galarneau at efficios.com>
Date: Thu Nov 6 17:34:08 2014 -0500
Fix: check userspace perf counter name when looking up contexts
create_ust_app_channel_context() looks for a context's existance
in a channel before adding it. However, it only checks for
context types. This is valid for all context types except for
LTTNG_UST_CONTEXT_PERF_THREAD_COUNTER since multiple perf
thread counters may be enabled at the same time.
This fix ensures that the perf counter name is taken into
consideration when checking for a context's presence in a
channel.
Reported-by: Alexander Grigoriev <alexgri at tbricks.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
Would you mind testing it? I'll backport it to the affected stable
branches if it fixes the problem.
Thanks!
Jérémie
>> > ua_ctx = alloc_ust_app_ctx(uctx);
>> > if (ua_ctx == NULL) {
>> > /* malloc failed */
>> > --
>> > 1.9.1
>> >
>> >
>> > _______________________________________________
>> > lttng-dev mailing list
>> > lttng-dev at lists.lttng.org
>> > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>
>>
>>
>> --
>> Jérémie Galarneau
>> EfficiOS Inc.
>> http://www.efficios.com
>
>
--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list