Hi Arnaldo, On Thu, Jun 08, 2017 at 11:05:32AM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Jun 08, 2017 at 10:40:44AM +0200, Thomas Richter escreveu: > > On s390 the counter and sampling facility do not support a > > precise IP skid level and sometimes returns EOPNOTSUPP when > > structure member precise_ip in struct perf_event_attr > > is not set to zero. > > > > On s390 commnd 'perf record -- true' fails with error EOPNOTSUPP. > > This happens only when no events are specified on command line. > > > > The functions called are > > ... > > --> perf_evlist__add_default > > --> perf_evsel__new_cycles > > --> perf_event_attr__set_max_precise_ip > > > > The last function determines the value of structure member precise_ip > > by invoking the perf_event_open() system call and checking the return code. > > The first successful open is the value for precise_ip. > > However the value is determined without setting member sample_period > > and indicates no sampling. > > On s390 the counter facility and sampling facility are different. > > The above procedure determines a precise_ip value of 3 using the > > counter facility. Later it uses the sampling facility with a value of 3 > > and fails with EOPNOTSUPP. > > > > Fix this by changing function perf_evsel__new_cycles(). It is called > > very early in the event setup. Delay the determination of > > the value of precise_ip until the context is known. This is the case > > when perf_evsel__config() is called. > > Function perf_evsel__new_cycles() just marks precise_ip to be > > determined later. > > > > Also change the modifier to 'P' for maximum detected precise level. > > This "Also" usually indicates that you are folding two changes into one > patch, please break it into two, and also send it to the perf That's usually right. Except for the change below. The perf_evsel__new_cycles() function constructs a perf event selection. The construction includes setting evsel->precise_max and specifying the event name string. To keep setting precise_max and the event name in sync, the :P modifier must be set. This should be part of the same commit and not split into two. > maintainers, I try to follow linux-perf-users, but you'll get usually > faster patch processing if you send to whoever is listed in MAINTAINERS. > > - Arnaldo > > > Suggested-by: Hendrik Brueckner <brueckner@xxxxxxxxxxxxxxxxxx> > > Signed-off-by: Thomas Richter <tmricht@xxxxxxxxxxxxxxxxxx> > > Reviewed-by: Hendrik Brueckner <brueckner@xxxxxxxxxxxxxxxxxx> > > --- > > tools/perf/util/evsel.c | 7 +++---- > > 1 files changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > > index ac59710..9266908 100644 > > --- a/tools/perf/util/evsel.c > > +++ b/tools/perf/util/evsel.c > > @@ -264,15 +264,14 @@ struct perf_evsel *perf_evsel__new_cycles(void) > > > > event_attr_init(&attr); > > > > - perf_event_attr__set_max_precise_ip(&attr); > > - > > evsel = perf_evsel__new(&attr); > > if (evsel == NULL) > > goto out; > > > > + evsel->precise_max = 1; > > + > > /* use asprintf() because free(evsel) assumes name is allocated */ > > - if (asprintf(&evsel->name, "cycles%.*s", > > - attr.precise_ip ? attr.precise_ip + 1 : 0, ":ppp") < 0) > > + if (asprintf(&evsel->name, "cycles:P") < 0) > > goto error_free; > > out: > > return evsel; > > -- > > 2.9.3 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > Kind regards, Hendrik -- Hendrik Brueckner brueckner@xxxxxxxxxxxxxxxxxx | IBM Deutschland Research & Development GmbH Linux on z Systems Development | Schoenaicher Str. 220, 71032 Boeblingen IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Koederitz Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
