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

Groups > linux.kernel > #1710092 > unrolled thread

[PATCH v2 05/19] perf, tools: Support weak groups

Started byAndi Kleen <andi@firstfloor.org>
First post2017-08-12 01:30 +0200
Last post2017-08-23 09:50 +0200
Articles 6 — 3 participants

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

  [PATCH v2 05/19] perf, tools: Support weak groups Andi Kleen <andi@firstfloor.org> - 2017-08-12 01:30 +0200
    Re: [PATCH v2 05/19] perf, tools: Support weak groups Jiri Olsa <jolsa@redhat.com> - 2017-08-22 10:40 +0200
      Re: [PATCH v2 05/19] perf, tools: Support weak groups Andi Kleen <ak@linux.intel.com> - 2017-08-22 23:10 +0200
    Re: [PATCH v2 05/19] perf, tools: Support weak groups Jiri Olsa <jolsa@redhat.com> - 2017-08-22 10:40 +0200
      Re: [PATCH v2 05/19] perf, tools: Support weak groups Andi Kleen <ak@linux.intel.com> - 2017-08-22 23:00 +0200
        Re: [PATCH v2 05/19] perf, tools: Support weak groups Jiri Olsa <jolsa@redhat.com> - 2017-08-23 09:50 +0200

#1710092 — [PATCH v2 05/19] perf, tools: Support weak groups

FromAndi Kleen <andi@firstfloor.org>
Date2017-08-12 01:30 +0200
Subject[PATCH v2 05/19] perf, tools: Support weak groups
Message-ID<udrxp-1lO-33@gated-at.bofh.it>
From: Andi Kleen <ak@linux.intel.com>

Setting up groups can be complicated due to the
complicated scheduling restrictions of different PMUs.
User tools usually don't understand all these restrictions.
Still in many cases it is useful to set up groups and
they work most of the time. However if the group
is set up wrong some members will not reported any values
because they never get scheduled.

Add a concept of a 'weak group': try to set up a group,
but if it's not schedulable fallback to not using
a group. That gives us the best of both worlds:
groups if they work, but still a usable fallback if they don't.

In theory it would be possible to have more complex fallback
strategies (e.g. try to split the group in half), but
the simple fallback of not using a group seems to work for now.

So far the weak group is only implemented for perf stat,
not for record.

Here's an unschedulable group (on IvyBridge with SMT on)

% perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}' -a sleep 1

        73,806,067      branches
         4,848,144      branch-misses             #    6.57% of all branches
        14,754,458      l1d.replacement
        24,905,558      l2_lines_in.all
   <not supported>      l2_rqsts.all_code_rd         <------- will never report anything

With the weak group:

% perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1

       125,366,055      branches                                                      (80.02%)
         9,208,402      branch-misses             #    7.35% of all branches          (80.01%)
        24,560,249      l1d.replacement                                               (80.00%)
        43,174,971      l2_lines_in.all                                               (80.05%)
        31,891,457      l2_rqsts.all_code_rd                                          (79.92%)

The extra event scheduled with some extra multiplexing

v2: Move fallback code to separate function.
Add comment on for_each_group_member
Adjust to new perf_evsel__close interface
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-list.txt |  1 +
 tools/perf/builtin-stat.c              | 35 ++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.h                |  1 +
 tools/perf/util/parse-events.c         |  8 +++++++-
 tools/perf/util/parse-events.l         |  2 +-
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index f709de54707b..d432965d728d 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -47,6 +47,7 @@ counted. The following modifiers exist:
  P - use maximum detected precise level
  S - read sample value (PERF_SAMPLE_READ)
  D - pin the event to the PMU
+ W - group is weak and will fallback to non-group if not schedulable
 
 The 'p' modifier can be used for specifying how precise the instruction
 address should be. The 'p' modifier can be specified multiple times:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 866da7aa54bf..ac605caf1a01 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
 	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
 }
 
+static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
+{
+	struct perf_evsel *c2, *leader;
+	bool is_open = true;
+
+	leader = counter->leader;
+	pr_debug("Weak group for %s/%d failed\n",
+			leader->name, counter->nr_members);
+
+	/*
+	 * for_each_group_member doesn't work here because it doesn't
+	 * include the first entry.
+	 */
+	evlist__for_each_entry(evsel_list, c2) {
+		if (c2 == counter)
+			is_open = false;
+		if (c2->leader == leader) {
+			if (is_open)
+				perf_evsel__close(c2);
+			c2->leader = c2;
+			c2->nr_members = 0;
+		}
+	}
+	return leader;
+}
+
 static int __run_perf_stat(int argc, const char **argv)
 {
 	int interval = stat_config.interval;
@@ -618,6 +644,15 @@ static int __run_perf_stat(int argc, const char **argv)
 	evlist__for_each_entry(evsel_list, counter) {
 try_again:
 		if (create_perf_stat_counter(counter) < 0) {
+
+			/* Weak group failed. Reset the group. */
+			if (errno == EINVAL &&
+			    counter->leader != counter &&
+			    counter->weak_group) {
+				counter = reset_weak_group(counter);
+				goto try_again;
+			}
+
 			/*
 			 * PPC returns ENXIO for HW counters until 2.6.37
 			 * (behavior changed with commit b0a873e).
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 351d3b2d8887..f538c3530227 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -136,6 +136,7 @@ struct perf_evsel {
 	const char *		metric_name;
 	struct perf_evsel	**metric_events;
 	bool			collect_stat;
+	bool			weak_group;
 };
 
 union u64_swap {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 84e301073885..cd89b5cba8d2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1358,6 +1358,7 @@ struct event_modifier {
 	int exclude_GH;
 	int sample_read;
 	int pinned;
+	int weak;
 };
 
 static int get_event_modifier(struct event_modifier *mod, char *str,
@@ -1376,6 +1377,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 
 	int exclude = eu | ek | eh;
 	int exclude_GH = evsel ? evsel->exclude_GH : 0;
+	int weak = 0;
 
 	memset(mod, 0, sizeof(*mod));
 
@@ -1413,6 +1415,8 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 			sample_read = 1;
 		} else if (*str == 'D') {
 			pinned = 1;
+		} else if (*str == 'W') {
+			weak = 1;
 		} else
 			break;
 
@@ -1443,6 +1447,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	mod->exclude_GH = exclude_GH;
 	mod->sample_read = sample_read;
 	mod->pinned = pinned;
+	mod->weak = weak;
 
 	return 0;
 }
@@ -1456,7 +1461,7 @@ static int check_modifier(char *str)
 	char *p = str;
 
 	/* The sizeof includes 0 byte as well. */
-	if (strlen(str) > (sizeof("ukhGHpppPSDI") - 1))
+	if (strlen(str) > (sizeof("ukhGHpppPSDIW") - 1))
 		return -1;
 
 	while (*p) {
@@ -1496,6 +1501,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
 		evsel->exclude_GH          = mod.exclude_GH;
 		evsel->sample_read         = mod.sample_read;
 		evsel->precise_max         = mod.precise_max;
+		evsel->weak_group	   = mod.weak;
 
 		if (perf_evsel__is_group_leader(evsel))
 			evsel->attr.pinned = mod.pinned;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index c42edeac451f..fdb5bb52f01f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -161,7 +161,7 @@ name		[a-zA-Z_*?][a-zA-Z0-9_*?.]*
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
 drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
-modifier_event	[ukhpPGHSDI]+
+modifier_event	[ukhpPGHSDIW]+
 modifier_bp	[rwx]{1,3}
 
 %%
-- 
2.9.4

[toc] | [next] | [standalone]

#1717183

FromJiri Olsa <jolsa@redhat.com>
Date2017-08-22 10:40 +0200
Message-ID<uhcT7-8f0-1@gated-at.bofh.it>
In reply to#1710092
On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Setting up groups can be complicated due to the
> complicated scheduling restrictions of different PMUs.
> User tools usually don't understand all these restrictions.
> Still in many cases it is useful to set up groups and
> they work most of the time. However if the group
> is set up wrong some members will not reported any values
> because they never get scheduled.
> 
> Add a concept of a 'weak group': try to set up a group,
> but if it's not schedulable fallback to not using
> a group. That gives us the best of both worlds:
> groups if they work, but still a usable fallback if they don't.
> 
> In theory it would be possible to have more complex fallback
> strategies (e.g. try to split the group in half), but
> the simple fallback of not using a group seems to work for now.
> 
> So far the weak group is only implemented for perf stat,
> not for record.
> 
> Here's an unschedulable group (on IvyBridge with SMT on)
> 
> % perf stat -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}' -a sleep 1
> 
>         73,806,067      branches
>          4,848,144      branch-misses             #    6.57% of all branches
>         14,754,458      l1d.replacement
>         24,905,558      l2_lines_in.all
>    <not supported>      l2_rqsts.all_code_rd         <------- will never report anything

also if I put 'cycles' instead of the l2_rqsts.all_code_rd,
I get clean open but 'not counted' as result.. I wonder
there's some counter scheduling issue

[root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W' -a sleep 1
Using CPUID GenuineIntel-6-3D
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
branches: 0 4004293853 0
branch-misses: 0 4004293853 0
l1d.replacement: 0 4004293853 0
l2_lines_in.all: 0 4004293853 0
cycles: 0 4004293853 0

 Performance counter stats for 'system wide':

     <not counted>      branches                                                      (0.00%)
     <not counted>      branch-misses                                                 (0.00%)
     <not counted>      l1d.replacement                                               (0.00%)
     <not counted>      l2_lines_in.all                                               (0.00%)
     <not counted>      cycles                                                        (0.00%)

       1.001088589 seconds time elapsed

jirka

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

#1717838

FromAndi Kleen <ak@linux.intel.com>
Date2017-08-22 23:10 +0200
Message-ID<uhoAV-7V4-3@gated-at.bofh.it>
In reply to#1717183
> also if I put 'cycles' instead of the l2_rqsts.all_code_rd,
> I get clean open but 'not counted' as result.. I wonder
> there's some counter scheduling issue
> 
> [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W' -a sleep 1
> Using CPUID GenuineIntel-6-3D
> l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> branches: 0 4004293853 0
> branch-misses: 0 4004293853 0
> l1d.replacement: 0 4004293853 0
> l2_lines_in.all: 0 4004293853 0
> cycles: 0 4004293853 0
> 
>  Performance counter stats for 'system wide':
> 
>      <not counted>      branches                                                      (0.00%)
>      <not counted>      branch-misses                                                 (0.00%)
>      <not counted>      l1d.replacement                                               (0.00%)
>      <not counted>      l2_lines_in.all                                               (0.00%)
>      <not counted>      cycles                                                        (0.00%)
> 
>        1.001088589 seconds time elapsed

Cannot reproduce on my system

Does it work without group or a normal group?

./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,cycles}:W'
Using CPUID GenuineIntel-6-3E
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
^Cbranches: 105004611 27393959413 27393959413
branch-misses: 5854172 27393959413 27393959413
l1d.replacement: 13935607 27393959413 27393959413
l2_lines_in.all: 24987238 27393959413 27393959413
cycles: 1259015810 27393959413 27393959413

 Performance counter stats for 'system wide':

       105,004,611      branches                                                    
         5,854,172      branch-misses             #    5.58% of all branches        
        13,935,607      l1d.replacement                                             
        24,987,238      l2_lines_in.all                                             
     1,259,015,810      cycles                                                      

       2.282966609 seconds time elapsed

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

#1717186

FromJiri Olsa <jolsa@redhat.com>
Date2017-08-22 10:40 +0200
Message-ID<uhcT7-8f0-13@gated-at.bofh.it>
In reply to#1710092
On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index f709de54707b..d432965d728d 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -47,6 +47,7 @@ counted. The following modifiers exist:
>   P - use maximum detected precise level
>   S - read sample value (PERF_SAMPLE_READ)
>   D - pin the event to the PMU
> + W - group is weak and will fallback to non-group if not schedulable
>  
>  The 'p' modifier can be used for specifying how precise the instruction
>  address should be. The 'p' modifier can be specified multiple times:
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 866da7aa54bf..ac605caf1a01 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
>  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
>  }
>  
> +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> +{
> +	struct perf_evsel *c2, *leader;
> +	bool is_open = true;
> +
> +	leader = counter->leader;
> +	pr_debug("Weak group for %s/%d failed\n",
> +			leader->name, counter->nr_members);

I'm getting 'branches/0' in here for you example:

[root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
Using CPUID GenuineIntel-6-3D
l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
Weak group for branches/0 failed

I'd expect nr_members == 5 ... any idea?

thanks,
jirka

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

#1717836

FromAndi Kleen <ak@linux.intel.com>
Date2017-08-22 23:00 +0200
Message-ID<uhorg-7Cb-5@gated-at.bofh.it>
In reply to#1717186
On Tue, Aug 22, 2017 at 10:34:15AM +0200, Jiri Olsa wrote:
> On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > index f709de54707b..d432965d728d 100644
> > --- a/tools/perf/Documentation/perf-list.txt
> > +++ b/tools/perf/Documentation/perf-list.txt
> > @@ -47,6 +47,7 @@ counted. The following modifiers exist:
> >   P - use maximum detected precise level
> >   S - read sample value (PERF_SAMPLE_READ)
> >   D - pin the event to the PMU
> > + W - group is weak and will fallback to non-group if not schedulable
> >  
> >  The 'p' modifier can be used for specifying how precise the instruction
> >  address should be. The 'p' modifier can be specified multiple times:
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 866da7aa54bf..ac605caf1a01 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
> >  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
> >  }
> >  
> > +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> > +{
> > +	struct perf_evsel *c2, *leader;
> > +	bool is_open = true;
> > +
> > +	leader = counter->leader;
> > +	pr_debug("Weak group for %s/%d failed\n",
> > +			leader->name, counter->nr_members);
> 
> I'm getting 'branches/0' in here for you example:
> 
> [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
> Using CPUID GenuineIntel-6-3D
> l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
> Weak group for branches/0 failed
> 
> I'd expect nr_members == 5 ... any idea?

Looks like nr_members is only correct in the leader. This works

@@ -599,7 +599,7 @@ static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
 
        leader = counter->leader;
        pr_debug("Weak group for %s/%d failed\n",
-                       leader->name, counter->nr_members);
+                       leader->name, leader->nr_members);


-Andi

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

#1718118

FromJiri Olsa <jolsa@redhat.com>
Date2017-08-23 09:50 +0200
Message-ID<uhyAl-66m-77@gated-at.bofh.it>
In reply to#1717836
On Tue, Aug 22, 2017 at 01:58:12PM -0700, Andi Kleen wrote:
> On Tue, Aug 22, 2017 at 10:34:15AM +0200, Jiri Olsa wrote:
> > On Fri, Aug 11, 2017 at 04:26:20PM -0700, Andi Kleen wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> > > index f709de54707b..d432965d728d 100644
> > > --- a/tools/perf/Documentation/perf-list.txt
> > > +++ b/tools/perf/Documentation/perf-list.txt
> > > @@ -47,6 +47,7 @@ counted. The following modifiers exist:
> > >   P - use maximum detected precise level
> > >   S - read sample value (PERF_SAMPLE_READ)
> > >   D - pin the event to the PMU
> > > + W - group is weak and will fallback to non-group if not schedulable
> > >  
> > >  The 'p' modifier can be used for specifying how precise the instruction
> > >  address should be. The 'p' modifier can be specified multiple times:
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index 866da7aa54bf..ac605caf1a01 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -582,6 +582,32 @@ static bool perf_evsel__should_store_id(struct perf_evsel *counter)
> > >  	return STAT_RECORD || counter->attr.read_format & PERF_FORMAT_ID;
> > >  }
> > >  
> > > +static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
> > > +{
> > > +	struct perf_evsel *c2, *leader;
> > > +	bool is_open = true;
> > > +
> > > +	leader = counter->leader;
> > > +	pr_debug("Weak group for %s/%d failed\n",
> > > +			leader->name, counter->nr_members);
> > 
> > I'm getting 'branches/0' in here for you example:
> > 
> > [root@krava perf]# ./perf stat -v -e '{branches,branch-misses,l1d.replacement,l2_lines_in.all,l2_rqsts.all_code_rd}:W' -a sleep 1
> > Using CPUID GenuineIntel-6-3D
> > l1d.replacement -> cpu/umask=0x1,period=2000003,event=0x51/
> > l2_lines_in.all -> cpu/umask=0x7,period=100003,event=0xf1/
> > l2_rqsts.all_code_rd -> cpu/umask=0xe4,period=200003,event=0x24/
> > Weak group for branches/0 failed
> > 
> > I'd expect nr_members == 5 ... any idea?
> 
> Looks like nr_members is only correct in the leader. This works
> 
> @@ -599,7 +599,7 @@ static struct perf_evsel *reset_weak_group(struct perf_evsel *counter)
>  
>         leader = counter->leader;
>         pr_debug("Weak group for %s/%d failed\n",
> -                       leader->name, counter->nr_members);
> +                       leader->name, leader->nr_members);
> 

right, that's it

jirka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web