From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Andrew Vagin <avagin@openvz.org>
Cc: linux-kernel@vger.kernel.org, Arun Sharma <asharma@fb.com>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
devel@openvz.org
Subject: Re: [PATCH 2/7] event: don't divide events if it has field period
Date: Wed, 09 Nov 2011 12:55:40 +0100 [thread overview]
Message-ID: <1320839740.13360.11.camel@twins> (raw)
In-Reply-To: <1320670457-2633428-3-git-send-email-avagin@openvz.org>
On Mon, 2011-11-07 at 15:54 +0300, Andrew Vagin wrote:
> 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.
It would be yes, but this code predates that, also it needs to work even
if the user doesn't provide 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 know how handle it.
Yeah, looks about right, would be awesome if we could strip some
branches out, but nothing obvious comes to mind.
> Signed-off-by: Andrew Vagin <avagin@openvz.org>
> ---
> kernel/events/core.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 12a0287..298702d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -4737,7 +4737,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);
>
> @@ -4771,6 +4770,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);
>
next prev parent reply other threads:[~2011-11-09 11:55 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-07 12:54 [PATCH 0/7] Profiling sleep times (v2) Andrew Vagin
2011-11-07 12:54 ` [PATCH 1/7] perf: use event_name() to get an event name Andrew Vagin
2011-11-07 12:54 ` [PATCH 2/7] event: don't divide events if it has field period Andrew Vagin
2011-11-09 11:55 ` Peter Zijlstra [this message]
2011-11-11 9:54 ` Andrew Vagin
2011-11-17 8:22 ` Andrew Wagin
2011-11-17 8:45 ` Peter Zijlstra
2011-11-18 23:36 ` [tip:perf/core] events: Don't " tip-bot for Andrew Vagin
2011-11-07 12:54 ` [PATCH 3/7] perf: add ability to record event period Andrew Vagin
2011-11-07 12:54 ` [PATCH 4/7] perf: add ability to change event according to sample Andrew Vagin
2011-11-07 21:14 ` David Ahern
2011-11-07 12:54 ` [PATCH 5/7] perf: teach "perf inject" to work with files Andrew Vagin
2011-11-07 12:54 ` [PATCH 6/7] perf: teach perf inject to merge sched_stat_* and sched_switch events Andrew Vagin
2011-11-07 12:54 ` [PATCH 7/7] perf: add scripts for profiling sleep times 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=1320839740.13360.11.camel@twins \
--to=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=asharma@fb.com \
--cc=avagin@openvz.org \
--cc=devel@openvz.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--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;
as well as URLs for NNTP newsgroup(s).