From patchwork Thu Sep 26 14:48:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Clark X-Patchwork-Id: 13813436 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 0EE9ECDE00E for ; Thu, 26 Sep 2024 14:53:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=f05PQzvOEJplO/uvLn+IJgwf9BEDklxbAmP9za94pBU=; b=GKHVTq/lcLynnu6YWhBCrGfpZG JvJ4zEPptvcLxnHkS2549grAGS+szMACZxAdcbixZC/FtnBXVxDdbyCIo3q/KxTDYBI6WEswnbxv/ dVlsPoXTwM4i1Qe62KwzCmVkZVfRPcDocQE+HEG5CFuNpasXPYShi9vAzj5UUFXWp+Obel+qxro8y dF8rEXFsJVND69Dk1+6Lz590wm2PYNDgTW8tOLRkvQnCtLPyXIw1Yo8vIo42gO/X+mA3RV4heTXsm Mnr698JEQLfktYna0Ws1HFVqQdOSuQ5xHtxDEQh334GHwQILa2k75GNtXbMCmWmRvd9U9bpGQa8Kv jsz+RXWw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1stprV-00000008f4A-0RWe; Thu, 26 Sep 2024 14:52:57 +0000 Received: from mail-ej1-x62a.google.com ([2a00:1450:4864:20::62a]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1stpoI-00000008eGk-32CU for linux-arm-kernel@lists.infradead.org; Thu, 26 Sep 2024 14:49:40 +0000 Received: by mail-ej1-x62a.google.com with SMTP id a640c23a62f3a-a8d3cde1103so150281966b.2 for ; Thu, 26 Sep 2024 07:49:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1727362177; x=1727966977; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=f05PQzvOEJplO/uvLn+IJgwf9BEDklxbAmP9za94pBU=; b=Fi5H3a4P7BGUj9D4azUozL3KHlCRNpmC+msLC2kLtoRD76ovGE0lgeoB0n5fxzpqmy 5/t9y5S4Mo9wdEm78IV2sXf4Z6dtztfmhWYke50v1II63LuPib+err9s296YVzRbf0Mr Yx7HTT/3EC8mvx+199Qigk2EmkTC2XrvNDzrJ6lhDm539cp9tnr8BHZqypGZyLRF8n2G 0CmEMCtaa7D//vbj6ChwWdoEAazD1YIA/Ge8DSu8t/Geq1YsUTk2Pztr9p6BFsF+cVyZ J7hPd2AdtxMgiJwPkxakdTRUUi+70vt7ckAknHLrBRxxEm/UlTSTcgK/SaXR0MjL129Q KxBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727362177; x=1727966977; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=f05PQzvOEJplO/uvLn+IJgwf9BEDklxbAmP9za94pBU=; b=JID+H6GijGke2H4SfrnwIBbW5FgKXzJ7DbqR8XlFGgwBIX2XM55EzXrUJyCtKcZq89 TuHeoHxnffKM2USMVNRJsM1npgo/5OO9Xz8KpknTwvyYYZ5UBzFRYWyGMUW3fykOTqt4 Ia8W5jcuqtKhMhk1zBgXOx7hyrjodL0kABrS1AgPHR3A7IRftp17PNCjTvfpKx8Xo8B7 ZQ7lTb5V1qWlaKeo6UbdGoXNs5JtcXBnNOFJO+iwXJkJzIIKfxI776ToNvxzFZe0ymcc YuaRHmC/tCrWp+H27/jGqdobyDyBkyeVhtsIEMZXVBpOlyD3y+yM+mW1aIrFCvjdpRug cZYA== X-Forwarded-Encrypted: i=1; AJvYcCXqgkYbXb0U4BtV2iBFsFKrJbBDD6QMgmK5i2fzqYk7t/h4zBSXBeG/5TjOmJ3CdxYKNBc31eHw2UYCqgAaJVIY@lists.infradead.org X-Gm-Message-State: AOJu0YwUQz2hoKKS4SGZRZUhffwRIXh6k8teQwtQtLU+IMS9tpdA/JwS Om7lCVaoOdcQbCcob7KPqxAah1piEuj2fj/Q2v5dPL3Q6xslZ99SnQv962w8Zgs= X-Google-Smtp-Source: AGHT+IFRodAK6m711faN788Vx2f57SE2Mt+eHRET+Mxtap6zJn+GDJXAKEY/OXvNgmlfFR++fXsaNw== X-Received: by 2002:a17:907:7b91:b0:a86:a56a:3596 with SMTP id a640c23a62f3a-a93a06a43f4mr529239266b.60.1727362177193; Thu, 26 Sep 2024 07:49:37 -0700 (PDT) Received: from localhost.localdomain ([89.47.253.130]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a93c294833asm5589966b.98.2024.09.26.07.49.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 07:49:36 -0700 (PDT) From: James Clark To: irogers@google.com, linux-perf-users@vger.kernel.org, kan.liang@linux.intel.com, ak@linux.intel.com, namhyung@kernel.org Cc: James Clark , John Garry , Will Deacon , Mike Leach , Leo Yan , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Weilin Wang , Dominique Martinet , Athira Rajeev , Colin Ian King , Yang Li , Yang Jihong , Howard Chu , Ze Gao , Sun Haiyong , Jing Zhang , Yicong Yang , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH v9 2/7] perf stat: Uniquify event name improvements Date: Thu, 26 Sep 2024 15:48:33 +0100 Message-Id: <20240926144851.245903-3-james.clark@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240926144851.245903-1-james.clark@linaro.org> References: <20240926144851.245903-1-james.clark@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240926_074938_774149_3166FF5B X-CRM114-Status: GOOD ( 24.97 ) 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 From: Ian Rogers Without aggregation on Intel: ``` $ perf stat -e instructions,cycles ... ``` Will use "cycles" for the name of the legacy cycles event but as "instructions" has a sysfs name it will and a "[cpu]" PMU suffix. This often breaks things as the space between the event and the PMU name look like an extra column. The existing uniquify logic was also uniquifying in cases when all events are core and not with uncore events, it was not correctly handling modifiers, etc. Change the logic so that an initial pass that can disable uniquification is run. For individual counters, disable uniquification in more cases such as for consistency with legacy events or for libpfm4 events. Don't use the "[pmu]" style suffix in uniquification, always use "pmu/.../". Change how modifiers/terms are handled in the uniquification so that they look like parse-able events. This fixes "102: perf stat metrics (shadow stat) test:" that has been failing due to "instructions [cpu]" breaking its column/awk logic when values aren't aggregated. This started happening when instructions could match a sysfs rather than a legacy event, so the fixes tag reflects this. Fixes: 617824a7f0f7 ("perf parse-events: Prefer sysfs/JSON hardware events over legacy") Acked-by: Namhyung Kim Signed-off-by: Ian Rogers [ Fix Intel TPEBS counting mode test ] Acked-by: Kan Liang Signed-off-by: James Clark --- .../perf/tests/shell/test_stat_intel_tpebs.sh | 11 +- tools/perf/util/stat-display.c | 101 ++++++++++++++---- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/tools/perf/tests/shell/test_stat_intel_tpebs.sh b/tools/perf/tests/shell/test_stat_intel_tpebs.sh index c60b29add980..9a11f42d153c 100755 --- a/tools/perf/tests/shell/test_stat_intel_tpebs.sh +++ b/tools/perf/tests/shell/test_stat_intel_tpebs.sh @@ -8,12 +8,15 @@ grep -q GenuineIntel /proc/cpuinfo || { echo Skipping non-Intel; exit 2; } # Use this event for testing because it should exist in all platforms event=cache-misses:R +# Hybrid platforms output like "cpu_atom/cache-misses/R", rather than as above +alt_name=/cache-misses/R + # Without this cmd option, default value or zero is returned -echo "Testing without --record-tpebs" -result=$(perf stat -e "$event" true 2>&1) -[[ "$result" =~ $event ]] || exit 1 +#echo "Testing without --record-tpebs" +#result=$(perf stat -e "$event" true 2>&1) +#[[ "$result" =~ $event || "$result" =~ $alt_name ]] || exit 1 # In platforms that do not support TPEBS, it should execute without error. echo "Testing with --record-tpebs" result=$(perf stat -e "$event" --record-tpebs -a sleep 0.01 2>&1) -[[ "$result" =~ "perf record" && "$result" =~ $event ]] || exit 1 +[[ "$result" =~ "perf record" && "$result" =~ $event || "$result" =~ $alt_name ]] || exit 1 diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c index ea96e4ebad8c..cbff43ff8d0f 100644 --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c @@ -871,38 +871,66 @@ static void printout(struct perf_stat_config *config, struct outstate *os, static void uniquify_event_name(struct evsel *counter) { - char *new_name; - char *config; - int ret = 0; + const char *name, *pmu_name; + char *new_name, *config; + int ret; - if (counter->uniquified_name || counter->use_config_name || - !counter->pmu_name || !strncmp(evsel__name(counter), counter->pmu_name, - strlen(counter->pmu_name))) + /* The evsel was already uniquified. */ + if (counter->uniquified_name) return; - config = strchr(counter->name, '/'); + /* Avoid checking to uniquify twice. */ + counter->uniquified_name = true; + + /* The evsel has a "name=" config term or is from libpfm. */ + if (counter->use_config_name || counter->is_libpfm_event) + return; + + /* Legacy no PMU event, don't uniquify. */ + if (!counter->pmu || + (counter->pmu->type < PERF_TYPE_MAX && counter->pmu->type != PERF_TYPE_RAW)) + return; + + /* A sysfs or json event replacing a legacy event, don't uniquify. */ + if (counter->pmu->is_core && counter->alternate_hw_config != PERF_COUNT_HW_MAX) + return; + + name = evsel__name(counter); + pmu_name = counter->pmu->name; + /* Already prefixed by the PMU name. */ + if (!strncmp(name, pmu_name, strlen(pmu_name))) + return; + + config = strchr(name, '/'); if (config) { - if (asprintf(&new_name, - "%s%s", counter->pmu_name, config) > 0) { - free(counter->name); - counter->name = new_name; - } - } else { - if (evsel__is_hybrid(counter)) { - ret = asprintf(&new_name, "%s/%s/", - counter->pmu_name, counter->name); + int len = config - name; + + if (config[1] == '/') { + /* case: event// */ + ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 2); } else { - ret = asprintf(&new_name, "%s [%s]", - counter->name, counter->pmu_name); + /* case: event/.../ */ + ret = asprintf(&new_name, "%s/%.*s,%s", pmu_name, len, name, config + 1); } + } else { + config = strchr(name, ':'); + if (config) { + /* case: event:.. */ + int len = config - name; - if (ret) { - free(counter->name); - counter->name = new_name; + ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 1); + } else { + /* case: event */ + ret = asprintf(&new_name, "%s/%s/", pmu_name, name); } } - - counter->uniquified_name = true; + if (ret > 0) { + free(counter->name); + counter->name = new_name; + } else { + /* ENOMEM from asprintf. */ + counter->uniquified_name = false; + } } static bool hybrid_uniquify(struct evsel *evsel, struct perf_stat_config *config) @@ -1559,6 +1587,31 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist print_metric_end(config, os); } +static void disable_uniquify(struct evlist *evlist) +{ + struct evsel *counter; + struct perf_pmu *last_pmu = NULL; + bool first = true; + + evlist__for_each_entry(evlist, counter) { + /* If PMUs vary then uniquify can be useful. */ + if (!first && counter->pmu != last_pmu) + return; + first = false; + if (counter->pmu) { + /* Allow uniquify for uncore PMUs. */ + if (!counter->pmu->is_core) + return; + /* Keep hybrid event names uniquified for clarity. */ + if (perf_pmus__num_core_pmus() > 1) + return; + } + } + evlist__for_each_entry_continue(evlist, counter) { + counter->uniquified_name = true; + } +} + void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *config, struct target *_target, struct timespec *ts, int argc, const char **argv) @@ -1572,6 +1625,8 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf .first = true, }; + disable_uniquify(evlist); + if (config->iostat_run) evlist->selected = evlist__first(evlist);