public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Stephane Eranian <eranian@google.com>
Cc: mingo@elte.hu, William Cohen <wcohen@redhat.com>,
	linux-kernel@vger.kernel.org, Arun Sharma <asharma@fb.com>,
	Vince Weaver <vince@deater.net>
Subject: Re: [RFC][PATCH 6/6] perf, tools: X86 RDPMC, RDTSC test
Date: Mon, 21 Nov 2011 17:59:27 +0100	[thread overview]
Message-ID: <1321894767.28118.14.camel@twins> (raw)
In-Reply-To: <1321889842.28118.9.camel@twins>

On Mon, 2011-11-21 at 16:37 +0100, Peter Zijlstra wrote:
> On Mon, 2011-11-21 at 16:29 +0100, Stephane Eranian wrote:
> > Peter,
> > 
> > I don't see how this test and infrastructure handles the case where the event
> > is multiplexed. I know there is time_enabled and time_running. But those are
> > not sync'd to the moment of the rdpmc(). I think there needs to be some other
> > timestamp in the mmap struct so the user can compute a delta to then add to
> > time_enabled and time_running.
> 
> When the counter isn't actually on the PMU, ->index will be 0 and rdpmc
> should not be attempted.
> 
> > Unless, we assume the two time metrics are there ONLY to compute a scaling
> > ratio. In which case, I think, we don't need the delta because if we
> > can do rdpmc()
> > it means the event is currently scheduled and thus time_enabled and time_running
> > are both ticking which means the scaling ratio does not change since the moment
> > the event was scheduled in.
> 
> Right, you don't need delta to compute the scale, but its useful for
> user-space time based measurements, Arun wanted to do something like
> that.

I'm full of crap, of course that makes a difference :-)

Even when both running and enabled are incremented, the scaling does
still change: 3/2 != 4/3 etc..

Using that we can actually deal with the whole multiplexing thing
without ever having to fall back to read(), something like:


static u64 mmap_read_self(void *addr)
{
        struct perf_event_mmap_page *pc = addr;
        u32 seq, idx, time_mult, time_shift;
        u64 count, cyc, time_offset, enabled, running, delta;

        do {
                seq = pc->lock;
                barrier();

                enabled = pc->time_enabled;
                running = pc->time_running;

                if (enabled != running) {
                        cyc = rdtsc();
                        time_mult = pc->time_mult;
                        time_shift = pc->time_shift;
                        time_offset = pc->time_offset;
                } 

                idx = pc->index;
                count = pc->offset;
                if (idx)
                        count += rdpmc(idx - 1);

                barrier();
        } while (pc->lock != seq);

        if (enabled != running) {
                u64 quot, rem;

                quot = (cyc >> time_shift);
                rem = cyc & ((1 << time_shift) - 1);
                delta = time_offset + quot * time_mult + 
                        ((rem * time_mult) >> time_shift);
                
                enabled += delta;
                if (idx)
                        running += delta;
                
                quot = count / running;
                rem = count % running;
                count = quot * enabled + (rem * enabled) / running;
        }

        return count;
}

Now all I need to do is make sure pc->offset actually makes sense,
because currently it looks like we're off by a factor
event->hw.prev_count when idx is set.




  reply	other threads:[~2011-11-21 16:59 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-21 14:51 [RFC][PATCH 0/6] perf: x86 RDPMC and RDTSC support Peter Zijlstra
2011-11-21 14:51 ` [RFC][PATCH 1/6] perf: Update the mmap control page on mmap() Peter Zijlstra
2011-11-21 14:51 ` [RFC][PATCH 2/6] perf, arch: Rework perf_event_index() Peter Zijlstra
2011-11-21 16:22   ` Eric B Munson
2011-11-21 17:23   ` Will Deacon
2011-11-21 19:18     ` Peter Zijlstra
2011-11-21 20:31       ` Will Deacon
2011-11-21 20:35         ` Peter Zijlstra
2011-11-21 22:43           ` Will Deacon
2011-11-22 11:26             ` Peter Zijlstra
2011-11-22 11:47               ` Will Deacon
2011-11-22 11:49                 ` Oleg Strikov
2011-11-22 11:52                   ` Will Deacon
2011-11-22 11:56                     ` Oleg Strikov
2011-11-22 12:00                     ` Oleg Strikov
2011-11-22 12:14                       ` Will Deacon
2011-11-22 12:25                         ` Oleg Strikov
2011-11-22 11:51                 ` Peter Zijlstra
2011-11-22 11:54                   ` Will Deacon
2011-11-22 11:48               ` Oleg Strikov
2011-11-21 14:51 ` [RFC][PATCH 3/6] perf, x86: Implement userspace RDPMC Peter Zijlstra
2011-11-21 14:51 ` [RFC][PATCH 4/6] perf, x86: Provide means of disabling " Peter Zijlstra
2011-11-21 14:51 ` [RFC][PATCH 5/6] perf: Extend the mmap control page with time (TSC) fields Peter Zijlstra
2011-12-28 17:55   ` Stephane Eranian
2011-11-21 14:51 ` [RFC][PATCH 6/6] perf, tools: X86 RDPMC, RDTSC test Peter Zijlstra
2011-11-21 15:29   ` Stephane Eranian
2011-11-21 15:37     ` Peter Zijlstra
2011-11-21 16:59       ` Peter Zijlstra [this message]
2011-11-21 17:42         ` Stephane Eranian
2011-11-21 15:02 ` [RFC][PATCH 0/6] perf: x86 RDPMC and RDTSC support Vince Weaver
2011-11-21 16:05   ` William Cohen
2011-11-21 16:08   ` William Cohen
2011-12-02 19:26 ` Arun Sharma
2011-12-02 22:22   ` Stephane Eranian
2011-12-05 20:16     ` Arun Sharma
2011-12-05 23:17       ` Arun Sharma
2011-12-06  1:38         ` Stephane Eranian
2011-12-06  9:42         ` Peter Zijlstra
2011-12-06 21:53           ` Arun Sharma
2011-12-16 22:36 ` Vince Weaver
2011-12-21 12:58   ` Peter Zijlstra
2011-12-21 13:15     ` Ingo Molnar
2011-12-23 20:12       ` Vince Weaver
2011-12-21 15:04     ` Vince Weaver
2011-12-21 21:32       ` Vince Weaver
2011-12-21 21:41         ` Peter Zijlstra
2011-12-21 22:19           ` Vince Weaver
2011-12-21 22:32             ` 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=1321894767.28118.14.camel@twins \
    --to=peterz@infradead.org \
    --cc=asharma@fb.com \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=vince@deater.net \
    --cc=wcohen@redhat.com \
    /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