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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 18A89C3F6B0 for ; Fri, 29 Jul 2022 16:13:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237156AbiG2QNW (ORCPT ); Fri, 29 Jul 2022 12:13:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38124 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236850AbiG2QNV (ORCPT ); Fri, 29 Jul 2022 12:13:21 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9836048C93; Fri, 29 Jul 2022 09:13:20 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F2962106F; Fri, 29 Jul 2022 09:13:20 -0700 (PDT) Received: from localhost.localdomain (H2XD2X12VG.cambridge.arm.com [10.1.36.148]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 946D23F73B; Fri, 29 Jul 2022 09:13:18 -0700 (PDT) From: =?UTF-8?q?Adri=C3=A1n=20Herrera=20Arcila?= To: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, acme@kernel.org Cc: peterz@infradead.org, mingo@redhat.com, mark.rutland@arm.com, alexander.shishkin@linux.intel.com, jolsa@kernel.org, namhyung@kernel.org, leo.yan@linaro.org, songliubraving@fb.com, james.clark@arm.com, =?UTF-8?q?Adri=C3=A1n=20Herrera=20Arcila?= Subject: [PATCH 2/2] perf stat: fix unexpected delay behaviour Date: Fri, 29 Jul 2022 16:12:44 +0000 Message-Id: <20220729161244.10522-2-adrian.herrera@arm.com> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220729161244.10522-1-adrian.herrera@arm.com> References: <20220729161244.10522-1-adrian.herrera@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org The described --delay behaviour is to delay the enablement of events, but not the execution of the command, if one is passed, which is incorrectly the current behaviour. This patch decouples the enablement from the delay, and enables events before or after launching the workload dependent on the options passed by the user. This code structure is inspired by that in perf-record, and tries to be consistent with it. Link: https://lore.kernel.org/linux-perf-users/7BFD066E-B0A8-49D4-B635-379328F0CF4C@fb.com Fixes: d0a0a511493d ("perf stat: Fix forked applications enablement of counters") Signed-off-by: Adrián Herrera Arcila --- tools/perf/builtin-stat.c | 56 ++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 318ffd119dad..f98c823b16dd 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -559,7 +559,7 @@ static bool handle_interval(unsigned int interval, int *times) return false; } -static int enable_counters(void) +static int enable_bpf_counters(void) { struct evsel *evsel; int err; @@ -572,28 +572,6 @@ static int enable_counters(void) if (err) return err; } - - if (stat_config.initial_delay < 0) { - pr_info(EVLIST_DISABLED_MSG); - return 0; - } - - if (stat_config.initial_delay > 0) { - pr_info(EVLIST_DISABLED_MSG); - usleep(stat_config.initial_delay * USEC_PER_MSEC); - } - - /* - * We need to enable counters only if: - * - we don't have tracee (attaching to task or cpu) - * - we have initial delay configured - */ - if (!target__none(&target) || stat_config.initial_delay) { - if (!all_counters_use_bpf) - evlist__enable(evsel_list); - if (stat_config.initial_delay > 0) - pr_info(EVLIST_ENABLED_MSG); - } return 0; } @@ -966,10 +944,24 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx) return err; } - err = enable_counters(); + err = enable_bpf_counters(); if (err) return -1; + /* + * Enable events manually here if perf-stat is run: + * 1. with a target (any of --all-cpus, --cpu, --pid or --tid) + * 2. without measurement delay (no --delay) + * 3. without all events associated to BPF + * + * This is because if run with a target, events are not enabled + * on exec if a workload is passed, and because there is no delay + * we ensure to enable them before the workload starts + */ + if (!target__none(&target) && !stat_config.initial_delay && + !all_counters_use_bpf) + evlist__enable(evsel_list); + /* Exec the command, if any */ if (forks) evlist__start_workload(evsel_list); @@ -977,6 +969,22 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx) t0 = rdclock(); clock_gettime(CLOCK_MONOTONIC, &ref_time); + /* + * If a measurement delay was specified, start it, and if positive, + * enable events manually after. We respect the delay even if all + * events are associated to BPF + */ + if (stat_config.initial_delay) { + /* At this point, events are guaranteed disabled */ + pr_info(EVLIST_DISABLED_MSG); + if (stat_config.initial_delay > 0) { + usleep(stat_config.initial_delay * USEC_PER_MSEC); + if (!all_counters_use_bpf) + evlist__enable(evsel_list); + pr_info(EVLIST_ENABLED_MSG); + } + } + if (forks) { if (interval || timeout || evlist__ctlfd_initialized(evsel_list)) status = dispatch_events(forks, timeout, interval, ×); -- 2.36.1