Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1361658 > unrolled thread

[tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events  into a cache_group tip-bot for Vikas Shivappa <tipbot@zytor.com> - 2016-03-21 11:00 +0100
    Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping  events into a cache_group Matt Fleming <matt@codeblueprint.co.uk> - 2016-03-21 16:00 +0100
      Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping  events into a cache_group Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2016-03-21 19:20 +0100
        Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping  events into a cache_group Matt Fleming <matt@codeblueprint.co.uk> - 2016-03-23 21:20 +0100
          Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping  events into a cache_group Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2016-03-24 00:00 +0100

#1361658 — [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

Commit-ID:  a223c1c7ab4cc64537dc4b911f760d851683768a
Gitweb:     http://git.kernel.org/tip/a223c1c7ab4cc64537dc4b911f760d851683768a
Author:     Vikas Shivappa <vikas.shivappa@linux.intel.com>
AuthorDate: Thu, 10 Mar 2016 15:32:07 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 21 Mar 2016 09:08:18 +0100

perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

Currently CQM (cache quality of service monitoring) is grouping all
events belonging to same PID to use one RMID. However its not counting
all of these different events. Hence we end up with a count of zero
for all events other than the group leader.

The patch tries to address the issue by keeping a flag in the
perf_event.hw which has other CQM related fields. The field is updated
at event creation and during grouping.

Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com>
[peterz: Changed hw_perf_event::is_group_event to an int]
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: fenghua.yu@intel.com
Cc: h.peter.anvin@intel.com
Cc: ravi.v.shankar@intel.com
Cc: vikas.shivappa@intel.com
Link: http://lkml.kernel.org/r/1457652732-4499-2-git-send-email-vikas.shivappa@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/cqm.c | 13 ++++++++++---
 include/linux/perf_event.h  |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/cqm.c b/arch/x86/events/intel/cqm.c
index 93cb412..b0226f1 100644
--- a/arch/x86/events/intel/cqm.c
+++ b/arch/x86/events/intel/cqm.c
@@ -281,9 +281,13 @@ static bool __match_event(struct perf_event *a, struct perf_event *b)
 
 	/*
 	 * Events that target same task are placed into the same cache group.
+	 * Mark it as a multi event group, so that we update ->count
+	 * for every event rather than just the group leader later.
 	 */
-	if (a->hw.target == b->hw.target)
+	if (a->hw.target == b->hw.target) {
+		b->hw.is_group_event = true;
 		return true;
+	}
 
 	/*
 	 * Are we an inherited event?
@@ -849,6 +853,7 @@ static void intel_cqm_setup_event(struct perf_event *event,
 	bool conflict = false;
 	u32 rmid;
 
+	event->hw.is_group_event = false;
 	list_for_each_entry(iter, &cache_groups, hw.cqm_groups_entry) {
 		rmid = iter->hw.cqm_rmid;
 
@@ -940,7 +945,9 @@ static u64 intel_cqm_event_count(struct perf_event *event)
 		return __perf_event_count(event);
 
 	/*
-	 * Only the group leader gets to report values. This stops us
+	 * Only the group leader gets to report values except in case of
+	 * multiple events in the same group, we still need to read the
+	 * other events.This stops us
 	 * reporting duplicate values to userspace, and gives us a clear
 	 * rule for which task gets to report the values.
 	 *
@@ -948,7 +955,7 @@ static u64 intel_cqm_event_count(struct perf_event *event)
 	 * specific packages - we forfeit that ability when we create
 	 * task events.
 	 */
-	if (!cqm_group_leader(event))
+	if (!cqm_group_leader(event) && !event->hw.is_group_event)
 		return 0;
 
 	/*
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 79ec7bb..7bb315b 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -121,6 +121,7 @@ struct hw_perf_event {
 		struct { /* intel_cqm */
 			int			cqm_state;
 			u32			cqm_rmid;
+			int			is_group_event;
 			struct list_head	cqm_events_entry;
 			struct list_head	cqm_groups_entry;
 			struct list_head	cqm_group_entry;

[toc] | [next] | [standalone]

#1361907 — Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

On Mon, 21 Mar, at 02:51:29AM, tip-bot for Vikas Shivappa wrote:
> Commit-ID:  a223c1c7ab4cc64537dc4b911f760d851683768a
> Gitweb:     http://git.kernel.org/tip/a223c1c7ab4cc64537dc4b911f760d851683768a
> Author:     Vikas Shivappa <vikas.shivappa@linux.intel.com>
> AuthorDate: Thu, 10 Mar 2016 15:32:07 -0800
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Mon, 21 Mar 2016 09:08:18 +0100
> 
> perf/x86/cqm: Fix CQM handling of grouping events into a cache_group
> 
> Currently CQM (cache quality of service monitoring) is grouping all
> events belonging to same PID to use one RMID. However its not counting
> all of these different events. Hence we end up with a count of zero
> for all events other than the group leader.
 
The reason that was done originally was because reporting for all events
in a group led to duplicate values, since you'd be emitting the same
RMID value multiple times.

Is this no longer a problem?

[toc] | [prev] | [next] | [standalone]

#1362064 — Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

On Mon, 21 Mar 2016, Matt Fleming wrote:

> On Mon, 21 Mar, at 02:51:29AM, tip-bot for Vikas Shivappa wrote:
>> Commit-ID:  a223c1c7ab4cc64537dc4b911f760d851683768a
>> Gitweb:     http://git.kernel.org/tip/a223c1c7ab4cc64537dc4b911f760d851683768a
>> Author:     Vikas Shivappa <vikas.shivappa@linux.intel.com>
>> AuthorDate: Thu, 10 Mar 2016 15:32:07 -0800
>> Committer:  Ingo Molnar <mingo@kernel.org>
>> CommitDate: Mon, 21 Mar 2016 09:08:18 +0100
>>
>> perf/x86/cqm: Fix CQM handling of grouping events into a cache_group
>>
>> Currently CQM (cache quality of service monitoring) is grouping all
>> events belonging to same PID to use one RMID. However its not counting
>> all of these different events. Hence we end up with a count of zero
>> for all events other than the group leader.
>
> The reason that was done originally was because reporting for all events
> in a group led to duplicate values, since you'd be emitting the same
> RMID value multiple times.
>
> Is this no longer a problem?

Before MBM , the below condition was never hit because we had only one event ?

-       if (a->hw.target == b->hw.target)
+       if (a->hw.target == b->hw.target) {
+               b->hw.is_group_event = true;

We are trying to address this for cases where different MBM(local or total) and 
cqm events are grouped into one RMID.

Which is the case which led to duplicate values ?

Thanks,
Vikas

>

[toc] | [prev] | [next] | [standalone]

#1363662 — Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

On Mon, 21 Mar, at 11:14:37AM, Vikas Shivappa wrote:
> 
> 
> Before MBM , the below condition was never hit because we had only one event ?
> 
> -       if (a->hw.target == b->hw.target)
> +       if (a->hw.target == b->hw.target) {
> +               b->hw.is_group_event = true;
> 
> We are trying to address this for cases where different MBM(local or total)
> and cqm events are grouped into one RMID.
 
I can't test these changes, so I'm only working from memory, but I
seem to recall that this condition is hit if monitoring simultaneously
from two invocations of perf. It's also possible to have pid/tid
groups overlapping, and that needs to be handled.

> Which is the case which led to duplicate values ?

Good question. Try monitoring a multithread process with these changes
and see if you get duplicate values reported.

[toc] | [prev] | [next] | [standalone]

#1363770 — Re: [tip:perf/urgent] perf/x86/cqm: Fix CQM handling of grouping events into a cache_group

On Wed, 23 Mar 2016, Matt Fleming wrote:

> On Mon, 21 Mar, at 11:14:37AM, Vikas Shivappa wrote:
>>
>>
>> Before MBM , the below condition was never hit because we had only one event ?
>>
>> -       if (a->hw.target == b->hw.target)
>> +       if (a->hw.target == b->hw.target) {
>> +               b->hw.is_group_event = true;
>>
>> We are trying to address this for cases where different MBM(local or total)
>> and cqm events are grouped into one RMID.
>
> I can't test these changes, so I'm only working from memory, but I
> seem to recall that this condition is hit if monitoring simultaneously
> from two invocations of perf. It's also possible to have pid/tid
> groups overlapping, and that needs to be handled.

Each task in a multithreaded process has an event. So it gets a different RMID. 
If two perf instances invoke 
the same pid then both of the instances expect to see the counters so its 
reported to both of them.

>
>> Which is the case which led to duplicate values ?
>
> Good question. Try monitoring a multithread process with these changes
> and see if you get duplicate values reported.

perf starts an event for each thread even when you give -p <process id>  a 
process which has multiple threads).
So it sends the pid of each thread to monitor and they get all seperate RMIDs. 
This should apply to the groups overlapping as well as this is dealing with only 
the perf task events..

>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web