From patchwork Sun Feb 19 09:28:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Rogers X-Patchwork-Id: 13145853 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9EC80C05027 for ; Sun, 19 Feb 2023 09:53:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:References: Mime-Version:Message-Id:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=A9EeheGklB840nENgQ8uDniGX3v0HXEyMNTZOHF7lS8=; b=heR34h0NLTvnog/k/6XdDrp69q RI8IGLcL3tzdG8R+273mujiLsCFT5GbttRwvzwiTbJ0fEsBv0yLQqCCUHElwu+G4MUo2DmlFz2chs b24k8dTvdpVpLDbj2OtXoqwb/jH6CFMfzQUlwT/HP2zMgG1A0+n+RbaoOwF7FlfJTjiLzViNR+MNB XfAVowIyas7UsRI4wOqPfyqkExCAEb7nz/pRtI/XlAybTunt7mE418mXWfo/EBLqx0coR/HMQOaVX o9YlnvqsrxA7LWYEx4mSTyu2SUqfCfOPldVad/KA5JscTVTDypQ3XDoK2wTIl1IEyfXjlxadhukb9 h4jH5ADA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pTgMk-001Cka-R2; Sun, 19 Feb 2023 09:52:20 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pTg8v-0017IF-Og for linux-arm-kernel@bombadil.infradead.org; Sun, 19 Feb 2023 09:38:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Content-Type:Cc:To:From:Subject: References:Mime-Version:Message-Id:In-Reply-To:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=c9cwoCmrhGIfqHNT3FoQmyx1P/u1tZpxQLcinvW6P6c=; b=IevtoYDXywTRzUqRzcUNYHR8jf z/7PUx0HArIUbAlB6jsfrLNnmWniBuHLEYZ74pm8OoczkPB0UmDncdetE17kLtPuUJZHlId6Ijk/j nrW9I2vzNw2ZFsfZAIIkduK3G1B/hFmHBTbq7fjHFdug2iw05v5JDwd4flbzkbVhS7zInLvotG1tl Y6dgE49kNtBEECbhiMIQPB8IafpqLkX9FKKEaDQ9sP3902sKX7AtlRgO9+NqKhWM2gOjbH/k6o7GH +FkvUrwO9MQgQ0gqTZ2kfwyKda15y39Mc6WZokcDIsGSh3s4pffku0RY8rkoi47H4KhmR4c4b2j9s ycLgaaSQ==; Received: from mail-pg1-x54a.google.com ([2607:f8b0:4864:20::54a]) by desiato.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pTg6v-00BMOJ-0b for linux-arm-kernel@lists.infradead.org; Sun, 19 Feb 2023 09:37:59 +0000 Received: by mail-pg1-x54a.google.com with SMTP id s11-20020a63524b000000b00502afcf62easo236104pgl.8 for ; Sun, 19 Feb 2023 01:35:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:references:mime-version:message-id:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=c9cwoCmrhGIfqHNT3FoQmyx1P/u1tZpxQLcinvW6P6c=; b=KZFTskEwL+dr/8fy74EfeXIcp2au/6QZJ3gGYkGbfhDs/uzHGlgkQlCyOtQqbhL3xY Jy1NQsoj7VKTWKAE/EitP649Ng/7FgcDlXOnxI1iU1cBlsH6DgyFo++fEl+UBe/E/+W5 6hC9CawjtpR0vb/YU+KI0IOuX1SnuuwXheTWvrToMTgJNwtFIZ16uN4w91EFgClU/Kzn OEa/fXjKSNwohQfBZOG6jV6XNsX9hHU7FRETX0HMhyg1Hxjj/M+tT5z24TCVCzcXtwH3 gj4FZtCyVBNx3v/vbbFuafTfVzEK/pt0egT6W2xLL818ez3k8/S5kDDMY5D9Auucjcbq xwzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:references:mime-version:message-id:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=c9cwoCmrhGIfqHNT3FoQmyx1P/u1tZpxQLcinvW6P6c=; b=jcd++721tbXCcGCvY6jDkOqgkFwx9ZOZrZSllh11onNWVUED+WP+h0reIo2bJqEenN wr0AKE0G2nLiVFzTMaxGZFlcOJglyO7lcu8byiMz99n2L8hfucdaN+EFW92rjLQzayZd oDcIXKCNOb74jlhys7JM6tPyjLK6KG58rCxD4eed21MNcPB9RlrTiPW0Egb/kA2IZfrp obJA4SLg5WVMPe5kGYeQdLL9tdyLNG8MPkf/rFfEvTmuBlWf2neqkaIfHhkTNnKOd18e YPlAbCbH9JF6glhzUwCz+7ityyDvy4Isx0a7VYeJSo0NtlgE49Jxs+ltvSKUioksagdb p0gQ== X-Gm-Message-State: AO0yUKU/2Y7Em4oyik2nmuuGDhGNJJE/qptreVY1MjPPURBX3sXnX0+Z m+5SvLko+zkbFOkQTOpHECi8A2ntHeMc X-Google-Smtp-Source: AK7set9bNoQNlEamDjQt4xlyLyaRtaDLRrmSUe5cS7+XDn8sfy7OhYKbMdzn4fSxiQ5++JQPwE6CfgAjpEI8 X-Received: from irogers.svl.corp.google.com ([2620:15c:2d4:203:cde9:3fbc:e1f1:6e3b]) (user=irogers job=sendgmr) by 2002:a17:902:ab0c:b0:198:fd60:df2c with SMTP id ik12-20020a170902ab0c00b00198fd60df2cmr28114plb.11.1676799353893; Sun, 19 Feb 2023 01:35:53 -0800 (PST) Date: Sun, 19 Feb 2023 01:28:46 -0800 In-Reply-To: <20230219092848.639226-1-irogers@google.com> Message-Id: <20230219092848.639226-50-irogers@google.com> Mime-Version: 1.0 References: <20230219092848.639226-1-irogers@google.com> X-Mailer: git-send-email 2.39.2.637.g21b0678d19-goog Subject: [PATCH v1 49/51] perf metric: Directly use counts rather than saved_value From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Maxime Coquelin , Alexandre Torgue , Zhengjun Xing , Sandipan Das , James Clark , Kajol Jain , John Garry , Kan Liang , Adrian Hunter , Andrii Nakryiko , Eduard Zingerman , Suzuki Poulouse , Leo Yan , Florian Fischer , Ravi Bangoria , Jing Zhang , Sean Christopherson , Athira Rajeev , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, Perry Taylor , Caleb Biggers Cc: Stephane Eranian , Ian Rogers X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230219_093757_564766_576347B5 X-CRM114-Status: GOOD ( 19.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Bugs with double aggregation have been introduced because of aggregation of counters and again with saved_value. Remove the generic metric use-case. Update parse-metric and pmu-events tests to update aggregate rather than saved_value counts. Signed-off-by: Ian Rogers --- tools/perf/tests/parse-metric.c | 4 +-- tools/perf/tests/pmu-events.c | 4 +-- tools/perf/util/stat-shadow.c | 56 +++++++++++---------------------- 3 files changed, 23 insertions(+), 41 deletions(-) diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c index 37e3371d978e..b9b8a48289c4 100644 --- a/tools/perf/tests/parse-metric.c +++ b/tools/perf/tests/parse-metric.c @@ -35,10 +35,10 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals) struct evsel *evsel; u64 count; - perf_stat__reset_shadow_stats(); + evlist__alloc_aggr_stats(evlist, 1); evlist__for_each_entry(evlist, evsel) { count = find_value(evsel->name, vals); - perf_stat__update_shadow_stats(evsel, count, 0); + evsel->stats->aggr->counts.val = count; if (!strcmp(evsel->name, "duration_time")) update_stats(&walltime_nsecs_stats, count); } diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c index 122e74c282a7..4ec2a4ca1a82 100644 --- a/tools/perf/tests/pmu-events.c +++ b/tools/perf/tests/pmu-events.c @@ -863,9 +863,9 @@ static int test__parsing_callback(const struct pmu_metric *pm, * zero when subtracted and so try to make them unique. */ k = 1; - perf_stat__reset_shadow_stats(); + evlist__alloc_aggr_stats(evlist, 1); evlist__for_each_entry(evlist, evsel) { - perf_stat__update_shadow_stats(evsel, k, 0); + evsel->stats->aggr->counts.val = k; if (!strcmp(evsel->name, "duration_time")) update_stats(&walltime_nsecs_stats, k); k++; diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c index 7b48e2bd3ba1..eba98520cea2 100644 --- a/tools/perf/util/stat-shadow.c +++ b/tools/perf/util/stat-shadow.c @@ -234,7 +234,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, int aggr_idx) { u64 count_ns = count; - struct saved_value *v; struct runtime_stat_data rsd = { .ctx = evsel_context(counter), .cgrp = counter->cgrp, @@ -265,19 +264,6 @@ void perf_stat__update_shadow_stats(struct evsel *counter, u64 count, update_runtime_stat(STAT_DTLB_CACHE, aggr_idx, count, &rsd); else if (evsel__match(counter, HW_CACHE, HW_CACHE_ITLB)) update_runtime_stat(STAT_ITLB_CACHE, aggr_idx, count, &rsd); - - if (counter->collect_stat) { - v = saved_value_lookup(counter, aggr_idx, true, STAT_NONE, 0, - rsd.cgrp); - update_stats(&v->stats, count); - if (counter->metric_leader) - v->metric_total += count; - } else if (counter->metric_leader && !counter->merged_stat) { - v = saved_value_lookup(counter->metric_leader, - aggr_idx, true, STAT_NONE, 0, rsd.cgrp); - v->metric_total += count; - v->metric_other++; - } } /* used for get_ratio_color() */ @@ -480,18 +466,17 @@ static int prepare_metric(struct evsel **metric_events, struct expr_parse_ctx *pctx, int aggr_idx) { - double scale; - char *n; - int i, j, ret; + int i; for (i = 0; metric_events[i]; i++) { - struct saved_value *v; - struct stats *stats; - u64 metric_total = 0; - int source_count; + char *n; + double val; + int source_count = 0; if (evsel__is_tool(metric_events[i])) { - source_count = 1; + struct stats *stats; + double scale; + switch (metric_events[i]->tool_event) { case PERF_TOOL_DURATION_TIME: stats = &walltime_nsecs_stats; @@ -515,35 +500,32 @@ static int prepare_metric(struct evsel **metric_events, pr_err("Unknown tool event '%s'", evsel__name(metric_events[i])); abort(); } + val = avg_stats(stats) * scale; + source_count = 1; } else { - v = saved_value_lookup(metric_events[i], aggr_idx, false, - STAT_NONE, 0, - metric_events[i]->cgrp); - if (!v) + struct perf_stat_evsel *ps = metric_events[i]->stats; + struct perf_stat_aggr *aggr = &ps->aggr[aggr_idx]; + + if (!aggr) break; - stats = &v->stats; + /* * If an event was scaled during stat gathering, reverse * the scale before computing the metric. */ - scale = 1.0 / metric_events[i]->scale; - + val = aggr->counts.val * (1.0 / metric_events[i]->scale); source_count = evsel__source_count(metric_events[i]); - - if (v->metric_other) - metric_total = v->metric_total * scale; } n = strdup(evsel__metric_id(metric_events[i])); if (!n) return -ENOMEM; - expr__add_id_val_source_count(pctx, n, - metric_total ? : avg_stats(stats) * scale, - source_count); + expr__add_id_val_source_count(pctx, n, val, source_count); } - for (j = 0; metric_refs && metric_refs[j].metric_name; j++) { - ret = expr__add_ref(pctx, &metric_refs[j]); + for (int j = 0; metric_refs && metric_refs[j].metric_name; j++) { + int ret = expr__add_ref(pctx, &metric_refs[j]); + if (ret) return ret; }