From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Vince Weaver <vweaver1@eecs.utk.edu>
Cc: linux-kernel@vger.kernel.org, mingo@elte.hu,
Paul Mackerras <paulus@samba.org>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
Stephane Eranian <eranian@gmail.com>
Subject: Re: [PATCH] perf_event use rdpmc rather than rdmsr when possible in kernel
Date: Mon, 27 Feb 2012 12:39:22 +0100 [thread overview]
Message-ID: <1330342762.11248.69.camel@twins> (raw)
In-Reply-To: <alpine.DEB.2.00.1202201722120.2753@cl320.eecs.utk.edu>
On Mon, 2012-02-20 at 17:38 -0500, Vince Weaver wrote:
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 5adce10..5550047 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -85,7 +85,7 @@ u64 x86_perf_event_update(struct perf_event *event)
> */
> again:
> prev_raw_count = local64_read(&hwc->prev_count);
> - rdmsrl(hwc->event_base, new_raw_count);
> + new_raw_count=native_read_pmc(hwc->event_base_rdpmc);
That really wants to be rdpmc(), bypassing paravirt like that isn't
nice.
>
> if (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
> new_raw_count) != prev_raw_count)
> @@ -768,9 +768,11 @@ static inline void x86_assign_hw_event(struct perf_event *event,
> } else if (hwc->idx >= X86_PMC_IDX_FIXED) {
> hwc->config_base = MSR_ARCH_PERFMON_FIXED_CTR_CTRL;
> hwc->event_base = MSR_ARCH_PERFMON_FIXED_CTR0 + (hwc->idx - X86_PMC_IDX_FIXED);
> + hwc->event_base_rdpmc = (hwc->idx - X86_PMC_IDX_FIXED) | 1<<30;
> } else {
> hwc->config_base = x86_pmu_config_addr(hwc->idx);
> hwc->event_base = x86_pmu_event_addr(hwc->idx);
> + hwc->event_base_rdpmc = x86_pmu_addr_offset(hwc->idx);
> }
> }
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index abb2776..432ac69 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -562,6 +562,7 @@ struct hw_perf_event {
> u64 last_tag;
> unsigned long config_base;
> unsigned long event_base;
> + unsigned long event_base_rdpmc;
We could make that unsigned int, the SDM explicitly states
rdmsr/wrmsr/rdpmc take ECX (and ignore the upper 32bits RCX).
> int idx;
> int last_cpu;
> struct hw_perf_event_extra extra_reg;
But yeah, avoiding the extra variable will add a conditional and extra
instructions to the rdpmc path.
next prev parent reply other threads:[~2012-02-27 11:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-20 22:38 [PATCH] perf_event use rdpmc rather than rdmsr when possible in kernel Vince Weaver
2012-02-27 11:39 ` Peter Zijlstra [this message]
2012-02-27 16:06 ` Vince Weaver
2012-02-27 16:18 ` Peter Zijlstra
2012-02-27 17:04 ` Vince Weaver
2012-03-01 22:28 Vince Weaver
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=1330342762.11248.69.camel@twins \
--to=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=eranian@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
--cc=vweaver1@eecs.utk.edu \
/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.