From: Peter Zijlstra <peterz@infradead.org>
To: Matt Fleming <matt@console-pimps.org>
Cc: Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Andi Kleen <andi@firstfloor.org>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Kanaka Juvva <kanaka.d.juvva@intel.com>,
Matt Fleming <matt.fleming@intel.com>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [PATCH 09/11] perf/x86/intel: Support task events with Intel CQM
Date: Fri, 7 Nov 2014 12:22:55 +0100 [thread overview]
Message-ID: <20141107112255.GB29390@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20141107100959.GK3592@console-pimps.org>
On Fri, Nov 07, 2014 at 10:09:59AM +0000, Matt Fleming wrote:
> On Fri, 07 Nov, at 10:08:04AM, Peter Zijlstra wrote:
> >
> > How is that supposed to work? You call __intel_cqm_event_count() on the
> > one cpu per socket, but then you use a local_add, not an atomic_add,
> > even though these adds can happen concurrently as per IPI broadcast.
>
> Ouch, right. That's broken.
>
> > Also, I think smp_call_function_many() ignores the current cpu, if this
> > cpu happens to be the cpu for this socket, you're up some creek without
> > no paddle, right?
>
> OK, I didn't realise that. Yeah that sounds very problematic. I think my
> eyes skipped over the word "other" in the smp_call_function_many() docs,
>
> * smp_call_function_many(): Run a function on a set of other CPUs.
>
> So, the correct way to do this is to iterate over cqm_cpumask and invoke
> smp_call_function_single(), right?
>
> > Thirdly, there is no serialization around calling perf_event_count() [or
> > your pmu::count method] so you cannot temporarily put it to 0.
>
> Urgh, thanks. Good spot. I'm gonna have to think of a suitable
> serialisation mechanism because all the current ones are pretty
> heavy-handed. And of course, there's the added fun that it needs to be
> held across the IPIs.
>
> Perhaps a per-cache-group mutex?
struct rmid_read {
unsigned int rmid;
atomic64_t value;
}
static void __remote_rmid_read(void *arg)
{
struct rmid_read *rr = arg;
u64 val = __rmid_read(rr->rmid);
if (val & (RMID_VAL_ERROR | RMID_VAL_UNAVAIL))
return;
atomic64_add(val, &rr->value);
}
static void task_rmid_read(struct perf_event *event)
{
struct rmid_read rr = {
.rmid = ...,
.value = ATOMIC64_INIT(0),
};
int cpu;
...
cpu = get_cpu();
smp_call_function_many(&cqm_cpumask, __remote_rmid_read, &rr);
if (cpumask_test(cpu, &cqm_cpumask)) {
local_irq_disable();
__remote_rmid_read(&rr);
local_irq_enable();
}
put_cpu();
local64_set(&event->count, atomic64_read(&rr->value));
}
next prev parent reply other threads:[~2014-11-07 11:23 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-06 12:23 [PATCH v3 00/11] perf: Intel Cache QoS Monitoring support Matt Fleming
2014-11-06 12:23 ` [PATCH 01/11] perf tools: Parse event per-package info files Matt Fleming
2014-11-06 12:23 ` [PATCH 02/11] perf tools: Implement snapshot event file logic Matt Fleming
2014-11-06 12:23 ` [PATCH 03/11] perf: Make perf_cgroup_from_task() global Matt Fleming
2014-11-06 12:23 ` [PATCH 04/11] perf: Add ->count() function to read per-package counters Matt Fleming
2014-11-06 12:23 ` [PATCH v3 05/11] perf: Move cgroup init before PMU ->event_init() Matt Fleming
2014-11-06 12:23 ` [PATCH 06/11] x86: Add support for Intel Cache QoS Monitoring (CQM) detection Matt Fleming
2014-11-06 12:23 ` [PATCH v3 07/11] perf/x86/intel: Add Intel Cache QoS Monitoring support Matt Fleming
2014-11-06 12:23 ` [PATCH 08/11] perf/x86/intel: Implement LRU monitoring ID allocation for CQM Matt Fleming
2014-11-06 12:23 ` [PATCH 09/11] perf/x86/intel: Support task events with Intel CQM Matt Fleming
2014-11-07 9:08 ` Peter Zijlstra
2014-11-07 10:09 ` Matt Fleming
2014-11-07 11:22 ` Peter Zijlstra [this message]
2014-11-06 12:23 ` [PATCH v3 10/11] perf/x86/intel: Perform rotation on Intel CQM RMIDs Matt Fleming
2014-11-07 12:06 ` Peter Zijlstra
2014-11-10 20:43 ` Matt Fleming
2014-11-10 20:58 ` Peter Zijlstra
2014-11-07 12:18 ` Peter Zijlstra
2014-11-10 20:50 ` Matt Fleming
2014-11-07 12:20 ` Peter Zijlstra
2014-11-10 20:56 ` Matt Fleming
2014-11-10 21:08 ` Peter Zijlstra
2014-11-07 12:34 ` Peter Zijlstra
2014-11-07 12:38 ` Peter Zijlstra
2014-11-14 12:35 ` Matt Fleming
2014-11-10 21:31 ` Matt Fleming
2014-11-11 9:37 ` Peter Zijlstra
2014-11-07 12:58 ` Peter Zijlstra
2014-11-06 12:23 ` [PATCH 11/11] perf/x86/intel: Enable conflicting event scheduling for CQM Matt Fleming
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=20141107112255.GB29390@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=andi@firstfloor.org \
--cc=hpa@zytor.com \
--cc=jolsa@redhat.com \
--cc=kanaka.d.juvva@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=matt.fleming@intel.com \
--cc=matt@console-pimps.org \
--cc=mingo@kernel.org \
--cc=tglx@linutronix.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;
as well as URLs for NNTP newsgroup(s).