All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andrew Vagin <avagin@openvz.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Paul Mackerras <paulus@samba.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Arun Sharma <asharma@fb.com>
Subject: Re: [PATCH] trace: add ability to set a target task for events (v2)
Date: Wed, 11 Jul 2012 17:12:04 +0200	[thread overview]
Message-ID: <1342019524.3462.167.camel@twins> (raw)
In-Reply-To: <1342018508.3462.163.camel@twins>

On Wed, 2012-07-11 at 16:55 +0200, Peter Zijlstra wrote:
> Right.. back when I did that the plan was to make PERF_SAMPLE_PERIOD fix
> that, of course that never seemed to have happened.
> 
> With PERF_SAMPLE_PERIOD you can simply write the 1b into the period of 1
> event and be done with it. 

It did! Andrew fixed it..

---
commit 5d81e5cfb37a174e8ddc0413e2e70cdf05807ace
Author: Andrew Vagin <avagin@openvz.org>
Date:   Mon Nov 7 15:54:12 2011 +0300

    events: Don't divide events if it has field period
    
    This patch solves the following problem:
    
    Now some samples may be lost due to throttling. The number of samples is
    restricted by sysctl_perf_event_sample_rate/HZ.  A trace event is
    divided on some samples according to event's period.  I don't sure, that
    we should generate more than one sample on each trace event. I think the
    better way to use SAMPLE_PERIOD.
    
    E.g.: I want to trace when a process sleeps. I created a process, which
    sleeps for 1ms and for 4ms.  perf got 100 events in both cases.
    
    swapper     0 [000]  1141.371830: sched_stat_sleep: comm=foo pid=1801 delay=1386750 [ns]
    swapper     0 [000]  1141.369444: sched_stat_sleep: comm=foo pid=1801 delay=4499585 [ns]
    
    In the first case a kernel want to send 4499585 events and
    in the second case it wants to send 1386750 events.
    perf-reports shows that process sleeps in both places equal time. It's
    bug.
    
    With this patch kernel generates one event on each "sleep" and the time
    slice is saved in the field "period". Perf knows how handle it.
    
    Signed-off-by: Andrew Vagin <avagin@openvz.org>
    Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Link: http://lkml.kernel.org/r/1320670457-2633428-3-git-send-email-avagin@openvz.org
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/kernel/events/core.c b/kernel/events/core.c
index eadac69..8d9dea5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4528,7 +4528,6 @@ static void perf_swevent_overflow(struct perf_event *event, u64 overflow,
 	struct hw_perf_event *hwc = &event->hw;
 	int throttle = 0;
 
-	data->period = event->hw.last_period;
 	if (!overflow)
 		overflow = perf_swevent_set_period(event);
 
@@ -4562,6 +4561,12 @@ static void perf_swevent_event(struct perf_event *event, u64 nr,
 	if (!is_sampling_event(event))
 		return;
 
+	if ((event->attr.sample_type & PERF_SAMPLE_PERIOD) && !event->attr.freq) {
+		data->period = nr;
+		return perf_swevent_overflow(event, 1, data, regs);
+	} else
+		data->period = event->hw.last_period;
+
 	if (nr == 1 && hwc->sample_period == 1 && !event->attr.freq)
 		return perf_swevent_overflow(event, 1, data, regs);
 


  parent reply	other threads:[~2012-07-11 15:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-11 14:14 Andrew Vagin
2012-07-11 14:31 ` Frederic Weisbecker
2012-07-11 14:33   ` Peter Zijlstra
2012-07-11 14:36     ` Frederic Weisbecker
2012-07-11 14:38       ` Peter Zijlstra
2012-07-11 14:48         ` Frederic Weisbecker
2012-07-11 14:55           ` Peter Zijlstra
2012-07-11 15:07             ` Frederic Weisbecker
2012-07-11 15:12             ` Peter Zijlstra [this message]
2012-07-11 15:17               ` Frederic Weisbecker
2012-07-11 15:09 ` Peter Zijlstra
2012-07-31 17:58 ` [tip:perf/urgent] perf/trace: Add ability to set a target task for events tip-bot for Andrew Vagin

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=1342019524.3462.167.camel@twins \
    --to=peterz@infradead.org \
    --cc=acme@ghostprotocols.net \
    --cc=asharma@fb.com \
    --cc=avagin@openvz.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --subject='Re: [PATCH] trace: add ability to set a target task for events (v2)' \
    /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

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.