mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: paulus@samba.org, mingo@elte.hu, linux-kernel@vger.kernel.org,
	oleg@redhat.com
Subject: Re: [PATCH] perf, core: Fix initial task_ctx/event installation
Date: Mon, 06 Jun 2011 19:40:26 +0200	[thread overview]
Message-ID: <1307382026.2322.226.camel@twins> (raw)
In-Reply-To: <1307371095-10141-1-git-send-email-jolsa@redhat.com>

It helps if you actually use the right email address for me ;-)

On Mon, 2011-06-06 at 16:38 +0200, Jiri Olsa wrote:
> hi,
> 
> I think I found a bug in the __perf_install_in_context. The attached
> patch fixies the problem for me, but I'm not sure it did not break
> anything else.. ;)
> 
> thanks,
> jirka
> 
> ---
> The __perf_install_in_context function does not install context
> events in case there's no active task context.
> 
> This might cause incorrect counts if application opens counter
> in its own task context. 
> 
> Following scenario leads to wrong counts:
> (this is the case for "perf test - test__open_syscall_event
> which counts sys_enter_open calls)
> 
> 1  - application is scheduled in
> 2  - application enters perf_event_open syscall with its own pid
> 3  - event/context is created
> 4  - __perf_install_in_context is called
> 5  - cpuctx->task_ctx is NULL
> 6  - perf_event_sched_in gets called with ctx == NULL, new event is not scheduled in
> 7  - application leaves the perf_event_open syscall
> 8  - application running code leading to increment the event counter
>      !!! this is where we lost the counts, since the event is not scheduled in !!!
> 9  - application is scheduled out
> 11 - application is scheduled in
> 12 - event is properly sceduled in
> 13 - event counter is now incremented
> 
> If the task is scheduled out and back in after the context is installed,
> but before it exits the perf_event_open syscall, the task_ctx gets
> properly set and event is properly scheduled in. In this case
> the perf test returns proper counts.
> 
> Attached patch changed this behaviour to install new task_ctx
> even if the current task_ctx is NULL.
> 
> Tested by succesfully running perf test command.

Right, so I have those bits as per https://lkml.org/lkml/2011/4/10/17,
they got lost in the commit due to a quilt refresh boo-boo.

With these bits it works on my dual core, but I still get lockups on my
24-cpu wsm machine.


  reply	other threads:[~2011-06-06 17:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-06 14:38 Jiri Olsa
2011-06-06 17:40 ` Peter Zijlstra [this message]
2011-06-06 18:27   ` Peter Zijlstra
2011-06-06 22:23     ` Peter Zijlstra
2011-06-06 22:28       ` Jiri Olsa
2011-06-07 12:10         ` Jiri Olsa
2011-06-07 12:03       ` [tip:perf/core] " tip-bot for Peter Zijlstra

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=1307382026.2322.226.camel@twins \
    --to=peterz@infradead.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=paulus@samba.org \
    /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

all inboxes | Powered by JetHome®