mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Joseph Schuchart <joseph.schuchart@tu-dresden.de>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	thomas.ilsche@tu-dresden.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Perf: Correct Assumptions about Sample Timestamps in Passes
Date: Fri, 03 Jan 2014 15:45:36 -0700	[thread overview]
Message-ID: <52C73D90.3020904@gmail.com> (raw)
In-Reply-To: <20140103220745.GB11061@localhost.localdomain>

On 1/3/14, 3:07 PM, Frederic Weisbecker wrote:
> On Wed, Jan 01, 2014 at 11:37:55AM -0700, David Ahern wrote:
>> On 12/26/13, 8:30 AM, Frederic Weisbecker wrote:
>>> On Thu, Dec 26, 2013 at 10:24:03AM -0500, David Ahern wrote:
>>>> On 12/26/13, 10:14 AM, Frederic Weisbecker wrote:
>>>>>> I was carrying that patch while working on perf-kvm-stat-live last
>>>>>> Fall. It does not solve the problem for live commands, so ended up
>>>>>> dropping it and going with local (to the command) hacks. I still
>>>>>> think for live commands getting a perf_clock timestamp at the start
>>>>>> of a round and using that as the flush time will work best.
>>
>> For perf-kvm-stat-live using perf_clock value at the start of the
>> round as the flush time works beautifully:
>>
>> https://github.com/dsahern/linux/commit/ba8b7b63d5dbdc95aedbbafa670c2232e0cc07a2
>>
>> Never once failed with "Warning: Timestamp below last timeslice
>> flush" error.
>
> I'm not sure I understand why we need that. Why doesn't it work by simply flushing
> events prior to the earliest timestamp among every CPUs last event?

Here's one scenario. Consider N-mmaps:

        |----- t_flush
        v
0   -----|---x------------------------
1   -----|----|------------------------
...      |
N   -----|-------ssss-|-----------------

      t_start t_1 ... t_N

You start a round at some time -- t_start. By starting a round it means 
you go to mmap 0 and check for events, then mmap 1, ..., mmap N. It 
takes a finite amount of time to move from one mmap to another.

Assume there are no events on mmap 0, 1, ... N-1 but samples are 
generated in mmap N. In the time it takes to move forward from 0 to N, a 
sample can be generated for mmap 0 and written to the buffer - the 'x' 
above. It now contains a timestamp < than samples on any other mmap and 
out pops the flush error.

perf-kvm can have over 650,000 events per second and those tend to come 
in bunches on a single mmap. So even if you go for a "max of the min 
times across mmaps" it is often wrong.

The non-perf_clock logic in perf-kvm uses the min time across all mmaps 
and even it occasionally fails with the flush error.

David


>
> I can see one remaining issue when an event interrupts another in a CPU. If the
> interrupt happens after perf_prepare_sample() -> perf_clock() and perf_output_begin(),
> we may have locally non-monotonic timestamps in a CPU buffer.
>
> That could be solved with a heuristic similar to yours: flush events prior a few millisecs
> before the barrier since interrupt are supposed to be short. Or we could move the perf_clock()
> event snapshot to perf_output_sample() to make sure that the event space is reserved before
> we get the timestamp, thus the interrupting events having superior timestamps are guaranteed
> to be past the interrupted event in the stream.
>


  reply	other threads:[~2014-01-03 22:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-14  8:07 Joseph Schuchart
2013-11-14  8:39 ` Ingo Molnar
2013-11-14  8:59   ` Joseph Schuchart
2013-11-14 10:05     ` Ingo Molnar
2013-11-14 14:26       ` David Ahern
2013-11-14 14:44         ` Peter Zijlstra
2013-11-14 15:02           ` David Ahern
2013-11-14 15:25             ` Peter Zijlstra
2013-11-21 14:55       ` Joseph Schuchart
2013-11-27 13:51         ` Ingo Molnar
2013-12-20 12:27           ` Joseph Schuchart
2013-12-20 17:09             ` David Ahern
2013-12-23 13:10               ` Frederic Weisbecker
2013-12-23 14:44                 ` David Ahern
2013-12-26 15:14                   ` Frederic Weisbecker
2013-12-26 15:24                     ` David Ahern
2013-12-26 15:30                       ` Frederic Weisbecker
2014-01-01 18:37                         ` David Ahern
2014-01-03 22:07                           ` Frederic Weisbecker
2014-01-03 22:45                             ` David Ahern [this message]
2014-01-04 15:05                               ` Frederic Weisbecker
2014-01-08 21:48                                 ` David Ahern
2014-01-09 15:19                                   ` Frederic Weisbecker
2014-01-12 15:46                                     ` David Ahern

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=52C73D90.3020904@gmail.com \
    --to=dsahern@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=fweisbec@gmail.com \
    --cc=joseph.schuchart@tu-dresden.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=thomas.ilsche@tu-dresden.de \
    /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

Powered by JetHome