All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pratyush Anand <panand@redhat.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	mingo@kernel.org, alexander.shishkin@linux.intel.com,
	eranian@google.com, linux-kernel@vger.kernel.org,
	vince@deater.net, dvyukov@google.com, andi@firstfloor.org,
	sasha.levin@oracle.com, oleg@redhat.com
Subject: Re: [RFC][PATCH 4/7] perf: Fix scaling vs enable_on_exec
Date: Tue, 23 Feb 2016 22:05:50 +0530	[thread overview]
Message-ID: <20160223163550.GC11724@dhcppc3.redhat.com> (raw)
In-Reply-To: <20160223154849.GC9102@krava.redhat.com>

Hi Peter/Jiri,

On 23/02/2016:04:48:49 PM, Jiri Olsa wrote:
> On Tue, Feb 23, 2016 at 04:27:29PM +0100, Peter Zijlstra wrote:
> > On Fri, Feb 19, 2016 at 03:37:47PM +0100, Peter Zijlstra wrote:
> > > Oleg reported that enable_on_exec results in weird scale factors.
> > > 
> > > The recent commit 3e349507d12d ("perf: Fix perf_enable_on_exec() event
> > > scheduling") caused this by moving task_ctx_sched_out() from before
> > > __perf_event_mask_enable() to after it.
> > > 
> > > The overlooked concequence of that change is that task_ctx_sched_out()
> > > would update the ctx time fields, and now __perf_event_mask_enable()
> > > uses stale time.
> > > 
> > > Fix this by adding an explicit time update.
> > > 
> > > While looking at this, I also found that we need an ctx->is_active
> > > check in perf_install_in_context().
> > > 
> > > XXX: does this actually fix the reported issue? I'm not sure what the
> > > reproduction case is. Also an earlier version made Jiri's machine
> > > explode -- something I've not managed to reproduce either.
> > 
> > Jiri, can you have a look at this and perhaps share the reproducer?
> 
> yep, I'm testing this patchset, but got stuck with 'crash' tool to get
> some reasonable output.. got stuck on unrelated sched deadlock ;-)
> 
> the reproducer is described in this email:
> http://marc.info/?l=linux-kernel&m=145568006709552&w=2
> 
> CC-ing Pratyush

Thanks for CCing.

Its better with this patch, still count is 1 more in case of higher probe hits (
like 65535 times).

Reproducer is:
---------------------------------------------------------
git clone https://github.com/rfmvh/perftool-testsuite.git
cd perftool-testsuite/base_probe/
./setup.sh 
for i in 1 2 3 103 997 65535; do
perf probe -x examples/exact_counts --add f_${i}x
done
perf stat -x';' -e 'probe_exact:*' examples/exact_counts
---------------------------------------------------------

I see following with above code:

65536;;probe_exact:f_65535x;84476560;100.00
997;;probe_exact:f_997x;84476560;100.00
103;;probe_exact:f_103x;84476560;100.00
3;;probe_exact:f_3x;84476560;100.00
2;;probe_exact:f_2x;84476560;100.00
1;;probe_exact:f_1x;84476560;100.00

~Pratyush

PS: Do I need to take all patches of series? Currently I have taken only this.

  reply	other threads:[~2016-02-23 16:35 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-19 14:37 [RFC][PATCH 0/7] perf: more fuzzer inspired patches Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 1/7] perf: Close install vs exit race Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 2/7] perf: Do not double free Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 3/7] perf: Allow perf_release() with !event->ctx Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 4/7] perf: Fix scaling vs enable_on_exec Peter Zijlstra
2016-02-23 15:27   ` Peter Zijlstra
2016-02-23 15:48     ` Jiri Olsa
2016-02-23 16:35       ` Pratyush Anand [this message]
2016-02-23 17:47         ` Peter Zijlstra
2016-02-24 11:53           ` Peter Zijlstra
2016-02-24 14:02             ` Peter Zijlstra
2016-02-24 16:02               ` Peter Zijlstra
2016-02-25  4:07                 ` Pratyush Anand
2016-02-23 21:44       ` Jiri Olsa
2016-02-26  2:25         ` Oleg Nesterov
2016-02-19 14:37 ` [RFC][PATCH 5/7] perf: Fix cloning Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 6/7] perf: Fix race between event install and jump_labels Peter Zijlstra
2016-02-19 14:37 ` [RFC][PATCH 7/7] perf: Cure event->pending_disable race 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=20160223163550.GC11724@dhcppc3.redhat.com \
    --to=panand@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=dvyukov@google.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sasha.levin@oracle.com \
    --cc=vince@deater.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.