From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0416C433E1 for ; Tue, 19 May 2020 20:15:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A98852075F for ; Tue, 19 May 2020 20:15:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="tFpDa+SI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727788AbgESUPz (ORCPT ); Tue, 19 May 2020 16:15:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54784 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726333AbgESUPz (ORCPT ); Tue, 19 May 2020 16:15:55 -0400 Received: from mail-qk1-x744.google.com (mail-qk1-x744.google.com [IPv6:2607:f8b0:4864:20::744]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DCF2AC08C5C1 for ; Tue, 19 May 2020 13:15:54 -0700 (PDT) Received: by mail-qk1-x744.google.com with SMTP id f13so1144160qkh.2 for ; Tue, 19 May 2020 13:15:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TO5dMGuoi1kCRRb/+6qO1E4s7zpB8nWv0o7765YzP7M=; b=tFpDa+SIF8J9hbLHG5GILLdJuWlP+vg72DAqF3ts8lf8v3hE4zugMieXVzFQflly/V TQAbRleTds+7OpbV0WPkI5sAWD9OKkAdjXJj7KakwQwWxf5dYRGRooPRl58kdcFySBKS Wx1lNQN604Mr6KzIv+ObErS6SOS6mpzTkW2TQ8g1D/sk6B62pLByagTT9UCqaE/rUd53 RXEyBeLTNqIPBFPJRWmocmSu92fWIWMMUY4aEdywN+mnjw7+HiDv6045xix+LFM4wvk2 ggFvva9bzwu20lIl+F+Exs+l5PyeF01FayycjxF5hfXLMIHBg2+/OU7tzpW6wxPLPLXN VbiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TO5dMGuoi1kCRRb/+6qO1E4s7zpB8nWv0o7765YzP7M=; b=Gkkj+dunyzNIQjlCeqGteaAMUCxWLzKjNCDgm1WC1vvjt7xH7wkLycFczE3c07cSeA WhotBLr6n0Zlm36LnKanilLvJWhu/V9+jcz1F5gSZ9u7U41bKCGQJJuBLg4UMhCVjubN wLCE73v2PDq+i1MQNOFY+y+jG6s3fKzTAZgXXarIY9cfkujPi4FaCy88zYqiWrwRfSpl 8HqBdM3ie2yNmK4KdjFSwhjaQheXQOPS6a+S5ijwMhOXIxzqdVIJ6RivblqD3otOkRgO CyPVdPw981mgRChKBtoRcGZuQUdip7AsrjTCfmJQZa09FDHv4/BKXW8nVED/ZQ319GTx Ta0g== X-Gm-Message-State: AOAM533b9pe40JTMVzSudThauryQC4JDWsNUn8S4+iWUhZeX2mcT8z4H kH3eWUkmWuKmVV1YvEfJS1ScONxt3VBHhZdPM8Tu5g== X-Google-Smtp-Source: ABdhPJyn2v4e6S+RNRBx13hLKpD3vGX1DVudWBoUdwMBu5NoaK5swkGvyds93T0lv3JKzDVZMnnKMw8fCOiZm3FUDow= X-Received: by 2002:a25:be81:: with SMTP id i1mr1771591ybk.184.1589919353706; Tue, 19 May 2020 13:15:53 -0700 (PDT) MIME-Version: 1.0 References: <20200515221732.44078-1-irogers@google.com> <20200515221732.44078-7-irogers@google.com> <20200519190602.GB28228@kernel.org> In-Reply-To: <20200519190602.GB28228@kernel.org> From: Ian Rogers Date: Tue, 19 May 2020 13:15:41 -0700 Message-ID: Subject: Re: [PATCH v3 6/7] perf test: Improve pmu event metric testing To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh , Kajol Jain , Andi Kleen , John Garry , Jin Yao , Kan Liang , Cong Wang , Kim Phillips , Adrian Hunter , Leo Yan , LKML , Networking , bpf , Stephane Eranian , Paul Clarke , Arnaldo Carvalho de Melo Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 19, 2020 at 12:06 PM Arnaldo Carvalho de Melo wrote: > > Em Fri, May 15, 2020 at 03:17:31PM -0700, Ian Rogers escreveu: > > Break pmu-events test into 2 and add a test to verify that all pmu > > metric expressions simply parse. Try to parse all metric ids/events, > > skip/warn if metrics for the current architecture fail to parse. To > > support warning for a skip, and an ability for a subtest to describe why > > it skips. > > > > Tested on power9, skylakex, haswell, broadwell, westmere, sandybridge and > > ivybridge. > > > > May skip/warn on other architectures if metrics are invalid. In > > particular s390 is untested, but its expressions are trivial. The > > untested architectures with expressions are power8, cascadelakex, > > tremontx, skylake, jaketown, ivytown and variants of haswell and > > broadwell. > > > > v3. addresses review comments from John Garry , > > Jiri Olsa and Arnaldo Carvalho de Melo > > . > > v2. changes the commit message as event parsing errors no longer cause > > the test to fail. > > > > Signed-off-by: Ian Rogers > > Cc: Adrian Hunter > > Cc: Alexander Shishkin > > Cc: Andi Kleen > > Cc: Jin Yao > > Cc: Jiri Olsa > > Cc: John Garry > > Cc: Kajol Jain > > Cc: Kan Liang > > Cc: Leo Yan > > Cc: Mark Rutland > > Cc: Namhyung Kim > > Cc: Paul Clarke > > Cc: Peter Zijlstra > > Cc: Stephane Eranian > > Link: http://lore.kernel.org/lkml/20200513212933.41273-1-irogers@google.com > > [ split from a larger patch ] > > Signed-off-by: Arnaldo Carvalho de Melo > > --- > > tools/perf/tests/builtin-test.c | 7 ++ > > tools/perf/tests/pmu-events.c | 168 ++++++++++++++++++++++++++++++-- > > tools/perf/tests/tests.h | 3 + > > 3 files changed, 172 insertions(+), 6 deletions(-) > > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > > index baee735e6aa5..9553f8061772 100644 > > --- a/tools/perf/tests/builtin-test.c > > +++ b/tools/perf/tests/builtin-test.c > > @@ -75,6 +75,13 @@ static struct test generic_tests[] = { > > { > > .desc = "PMU events", > > .func = test__pmu_events, > > + .subtest = { > > + .skip_if_fail = false, > > + .get_nr = test__pmu_events_subtest_get_nr, > > + .get_desc = test__pmu_events_subtest_get_desc, > > + .skip_reason = test__pmu_events_subtest_skip_reason, > > + }, > > + > > }, > > { > > .desc = "DSO data read", > > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c > > index d64261da8bf7..e21f0addcfbb 100644 > > --- a/tools/perf/tests/pmu-events.c > > +++ b/tools/perf/tests/pmu-events.c > > @@ -8,6 +8,9 @@ > > #include > > #include "debug.h" > > #include "../pmu-events/pmu-events.h" > > +#include "util/evlist.h" > > +#include "util/expr.h" > > +#include "util/parse-events.h" > > > > struct perf_pmu_test_event { > > struct pmu_event event; > > @@ -144,7 +147,7 @@ static struct pmu_events_map *__test_pmu_get_events_map(void) > > } > > > > /* Verify generated events from pmu-events.c is as expected */ > > -static int __test_pmu_event_table(void) > > +static int test_pmu_event_table(void) > > { > > struct pmu_events_map *map = __test_pmu_get_events_map(); > > struct pmu_event *table; > > @@ -347,14 +350,11 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count) > > return res; > > } > > > > -int test__pmu_events(struct test *test __maybe_unused, > > - int subtest __maybe_unused) > > + > > +static int test_aliases(void) > > { > > struct perf_pmu *pmu = NULL; > > > > - if (__test_pmu_event_table()) > > - return -1; > > - > > while ((pmu = perf_pmu__scan(pmu)) != NULL) { > > int count = 0; > > > > @@ -377,3 +377,159 @@ int test__pmu_events(struct test *test __maybe_unused, > > > > return 0; > > } > > + > > +static bool is_number(const char *str) > > +{ > > + char *end_ptr; > > + > > + strtod(str, &end_ptr); > > + return end_ptr != str; > > +} > > So, this breaks in some systems: > > cc1: warnings being treated as errors > tests/pmu-events.c: In function 'is_number': > tests/pmu-events.c:385: error: ignoring return value of 'strtod', declared with attribute warn_unused_result > mv: cannot stat `/tmp/build/perf/tests/.pmu-events.o.tmp': No such file or director > > So I'm changing it to verify the result of strtod() which is, humm, > interesting, please check: Thanks Arnaldo and sorry for the difficulty. This looks like a good fix. > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c > index 3de59564deb0..6c58c3a89e6b 100644 > --- a/tools/perf/tests/pmu-events.c > +++ b/tools/perf/tests/pmu-events.c > @@ -1,4 +1,5 @@ > // SPDX-License-Identifier: GPL-2.0 > +#include "math.h" > #include "parse-events.h" > #include "pmu.h" > #include "tests.h" > @@ -381,8 +382,12 @@ static int test_aliases(void) > static bool is_number(const char *str) > { > char *end_ptr; > + double v; > > - strtod(str, &end_ptr); > + errno = 0; > + v = strtod(str, &end_ptr); > + if ((errno == ERANGE && (v == HUGE_VAL || v == -HUGE_VAL)) || (errno != 0 && v == 0.0)) errno can either be 0 or ERANGE here, but we test both. Perhaps use errno != 0 for both cases as the man page notes suggest doing this. The tests using v are necessary to avoid the unused result, but presumably any errno case should return false here? I guess testing that is redundant as the return below will catch it. Perhaps this should be: errno = 0; v = strtod(str, &end_ptr); (void)v; /* We don't care for the value of the double, just that it converts. Avoid unused result warnings. */ return errno == 0 && end_ptr != str; Thanks, Ian > + return false; > return end_ptr != str; > } >