Message ID | 20231031120526.11502-2-nick.forrington@arm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b90f:0:b0:403:3b70:6f57 with SMTP id t15csp185612vqg; Tue, 31 Oct 2023 05:06:24 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEubMG3EQYj+adD8iDvM+oYaxNKoTPNoJdZxLCPTBxSgbKToqieuDZnPpp4Uk/0WTpreNmk X-Received: by 2002:a05:6a20:42a5:b0:14d:6309:fc96 with SMTP id o37-20020a056a2042a500b0014d6309fc96mr12494910pzj.4.1698753984582; Tue, 31 Oct 2023 05:06:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698753984; cv=none; d=google.com; s=arc-20160816; b=ranZwVvwBv+Hx9/pz92Ttg3D+Amug1ku6bS0EJ8vgWHuRdt7W7on/g6IEk+hJSXi7d JGVfYKFeODCVINMGukyAw6fe34tuKPFaaAJZdABjTPZW6FAzexln3dYbBKmUvdmlBm3l 9dMF8vCUegA5WC3D982IZnI57E5Beq9DH8FUC2m0wkrC4dd0LhuA4b0c0ypcG/Cea6iT IbpatN2nER0WGLWM+NWNV64gTxOu+LUyy+BYY8PWuOumTSvGsqpbt4n7Pb/IVr1qrwmI AxcHQhSPV8jznfyqiBWPl7V0ozWza9o0bo3R7HnyCpXVcmx7N8P5R/q+CIHeyl9bYx8A Aldg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=IqXxlomYcWKX0hQKUgJt4SwcyuWHy79jBpPSbjSdYIQ=; fh=Dk4XfLMM0bGHqAzAqXdX1dBFvMRKfVJ5S146XXkq5QM=; b=T1zyfngMFbZNqLU07GsFNSXklueu+cHysOoyOwyW2vIfswYVcVjKCsb14fgOJOGQjP TXo+umxMIKPTuE2KIPdO1wYso57KBdQTXih+Xwhw8noA0PxLgWOW9kAQk0OLnIeCg3rJ IFf+5vOecYyz67NhPWHQDcEaEQEgwy07099H9Y2xIw2OrH4UMptgGW5JT9bJmSUdY/D3 zlCE5NQU101OlUs6FnsRQyeJr0C0yzJ3pyvvyTCbNrCuaILciPejO/9nGqNwkFiRiXDn SxLTTLuOzZR0WIdjiUskf3VFSuricA7CRpMWmHybUVxm4fqYMS3ATQxE/LfUsIUTIZYv b1xA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id m7-20020a633f07000000b005b95ccd1b4dsi937927pga.82.2023.10.31.05.06.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Oct 2023 05:06:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 4FC8580A8BA7; Tue, 31 Oct 2023 05:06:09 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344183AbjJaMFv (ORCPT <rfc822;chrisjones.unixmen@gmail.com> + 33 others); Tue, 31 Oct 2023 08:05:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50650 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344163AbjJaMFs (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 31 Oct 2023 08:05:48 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id BC5155595; Tue, 31 Oct 2023 05:05:44 -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 1F8A3DA7; Tue, 31 Oct 2023 05:06:26 -0700 (PDT) Received: from dsg-hive-n1sdp-01.. (dsg-hive-n1sdp-01.cambridge.arm.com [10.2.2.18]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id DA45E3F738; Tue, 31 Oct 2023 05:05:42 -0700 (PDT) From: Nick Forrington <nick.forrington@arm.com> To: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org Cc: Nick Forrington <nick.forrington@arm.com>, stable@kernel.org, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, Arnaldo Carvalho de Melo <acme@redhat.com> Subject: [PATCH 1/2] perf lock report: Restore aggregation by caller by default Date: Tue, 31 Oct 2023 12:05:24 +0000 Message-ID: <20231031120526.11502-2-nick.forrington@arm.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231031120526.11502-1-nick.forrington@arm.com> References: <20231031120526.11502-1-nick.forrington@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Tue, 31 Oct 2023 05:06:09 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781272658148303699 X-GMAIL-MSGID: 1781272658148303699 |
Series |
Perf lock improvements
|
|
Commit Message
Nick Forrington
Oct. 31, 2023, 12:05 p.m. UTC
This change restores the previous default behaviour for "perf lock
report", making the current aggregate-by-address behaviour available via
the new "--lock-addr" command line parameter.
This makes the behaviour consistent with "perf lock contention" (which
also aggregates by caller by default, or by address when "--lock-addr"
is specified).
Commit 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option")
introduced aggregation modes for "perf lock contention" and (potentially
inadvertently) changed the behaviour of "perf lock report" from
aggregate-by-caller to aggregate-by-address (making the prior behaviour
inaccessible).
Example aggregate-by-address output:
$ perf lock report -F acquired
Name acquired
event_mutex 34
21
1
Example aggregate-by-caller output:
$ perf lock report -F acquired
Name acquired
perf_trace_init+... 34
lock_mm_and_find... 20
inherit_event.co... 1
do_madvise+0x1f8 1
Cc: stable@kernel.org
Fixes: 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option")
Signed-off-by: Nick Forrington <nick.forrington@arm.com>
---
tools/perf/Documentation/perf-lock.txt | 4 ++++
tools/perf/builtin-lock.c | 24 +++++++++++++++++++++---
2 files changed, 25 insertions(+), 3 deletions(-)
Comments
On 31/10/2023 12:05, Nick Forrington wrote: > This change restores the previous default behaviour for "perf lock > report", making the current aggregate-by-address behaviour available via > the new "--lock-addr" command line parameter. > > This makes the behaviour consistent with "perf lock contention" (which > also aggregates by caller by default, or by address when "--lock-addr" > is specified). > > Commit 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") > introduced aggregation modes for "perf lock contention" and (potentially > inadvertently) changed the behaviour of "perf lock report" from > aggregate-by-caller to aggregate-by-address (making the prior behaviour > inaccessible). > > Example aggregate-by-address output: > > $ perf lock report -F acquired > Name acquired > > event_mutex 34 > 21 > 1 > > Example aggregate-by-caller output: > > $ perf lock report -F acquired > Name acquired > > perf_trace_init+... 34 > lock_mm_and_find... 20 > inherit_event.co... 1 > do_madvise+0x1f8 1 > > Cc: stable@kernel.org > Fixes: 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") > Signed-off-by: Nick Forrington <nick.forrington@arm.com> > --- > tools/perf/Documentation/perf-lock.txt | 4 ++++ > tools/perf/builtin-lock.c | 24 +++++++++++++++++++++--- > 2 files changed, 25 insertions(+), 3 deletions(-) > Reviewed-by: James Clark <james.clark@arm.com> > diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt > index 503abcba1438..349333acbbfc 100644 > --- a/tools/perf/Documentation/perf-lock.txt > +++ b/tools/perf/Documentation/perf-lock.txt > @@ -80,6 +80,10 @@ REPORT OPTIONS > --combine-locks:: > Merge lock instances in the same class (based on name). > > +-l:: > +--lock-addr:: > + Show lock contention stat by address > + > -t:: > --threads:: > The -t option is to show per-thread lock stat like below: > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index fa7419978353..3aa8ba5ad928 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -78,7 +78,7 @@ struct callstack_filter { > > static struct lock_filter filters; > > -static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR; > +static enum lock_aggr_mode aggr_mode = LOCK_AGGR_CALLER; > > static bool needs_callstack(void) > { > @@ -1983,8 +1983,8 @@ static int __cmd_report(bool display_info) > if (select_key(false)) > goto out_delete; > > - if (show_thread_stats) > - aggr_mode = LOCK_AGGR_TASK; > + aggr_mode = show_thread_stats ? LOCK_AGGR_TASK : > + show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER; > > err = perf_session__process_events(session); > if (err) > @@ -2008,6 +2008,19 @@ static void sighandler(int sig __maybe_unused) > { > } > > +static int check_lock_report_options(const struct option *options, > + const char * const *usage) > +{ > + if (show_thread_stats && show_lock_addrs) { > + pr_err("Cannot use thread and addr mode together\n"); > + parse_options_usage(usage, options, "threads", 0); > + parse_options_usage(NULL, options, "lock-addr", 0); > + return -1; > + } > + > + return 0; > +} > + > static int check_lock_contention_options(const struct option *options, > const char * const *usage) > > @@ -2589,6 +2602,7 @@ int cmd_lock(int argc, const char **argv) > /* TODO: type */ > OPT_BOOLEAN('c', "combine-locks", &combine_locks, > "combine locks in the same class"), > + OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"), > OPT_BOOLEAN('t', "threads", &show_thread_stats, > "show per-thread lock stats"), > OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"), > @@ -2680,6 +2694,10 @@ int cmd_lock(int argc, const char **argv) > if (argc) > usage_with_options(report_usage, report_options); > } > + > + if (check_lock_report_options(report_options, report_usage) < 0) > + return -1; > + > rc = __cmd_report(false); > } else if (!strcmp(argv[0], "script")) { > /* Aliased to 'perf script' */
Hello, On Tue, Oct 31, 2023 at 5:05 AM Nick Forrington <nick.forrington@arm.com> wrote: > > This change restores the previous default behaviour for "perf lock > report", making the current aggregate-by-address behaviour available via > the new "--lock-addr" command line parameter. > > This makes the behaviour consistent with "perf lock contention" (which > also aggregates by caller by default, or by address when "--lock-addr" > is specified). I understand your concern but actually there's a difference. "perf lock contention" is a new command which works with new contention tracepoints whereas "perf lock report" works with old lockdep/lockstat tracepoints which are not available in the default configuration. I made "perf lock contention" compatible to "perf lock report" so it mimics the old tracepoints behavior as much as possible using new tracepoints. But the important difference is that new contention tracepoints don't have lock names. The old perf lock report showed lock names by default but contention output had to use the caller instead. > > Commit 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") > introduced aggregation modes for "perf lock contention" and (potentially > inadvertently) changed the behaviour of "perf lock report" from > aggregate-by-caller to aggregate-by-address (making the prior behaviour > inaccessible). So it doesn't change the behavior of perf lock report. You're adding a new (default) feature for perf lock report to sort the output by caller. And please note that caller info needs callstacks. perf lock record adds it by default when it finds there are only lock contention tracepoints. But if it really has the old tracepoints, caller won't work unless you enabled callstack collection manually (-g). > > Example aggregate-by-address output: > > $ perf lock report -F acquired I guess you need -l option here. > Name acquired > > event_mutex 34 > 21 > 1 This is because you used contention tracepoints and they don't have lock names. > > Example aggregate-by-caller output: > > $ perf lock report -F acquired > Name acquired > > perf_trace_init+... 34 > lock_mm_and_find... 20 > inherit_event.co... 1 > do_madvise+0x1f8 1 Maybe it's ok to change the default behavior for contention tracepoints. But when lockdep tracepoints are available it should use the existing addr (symbol) mode. > > Cc: stable@kernel.org > Fixes: 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") So, I don't think this is a fix. Thanks, Namhyung > Signed-off-by: Nick Forrington <nick.forrington@arm.com> > --- > tools/perf/Documentation/perf-lock.txt | 4 ++++ > tools/perf/builtin-lock.c | 24 +++++++++++++++++++++--- > 2 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt > index 503abcba1438..349333acbbfc 100644 > --- a/tools/perf/Documentation/perf-lock.txt > +++ b/tools/perf/Documentation/perf-lock.txt > @@ -80,6 +80,10 @@ REPORT OPTIONS > --combine-locks:: > Merge lock instances in the same class (based on name). > > +-l:: > +--lock-addr:: > + Show lock contention stat by address > + > -t:: > --threads:: > The -t option is to show per-thread lock stat like below: > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c > index fa7419978353..3aa8ba5ad928 100644 > --- a/tools/perf/builtin-lock.c > +++ b/tools/perf/builtin-lock.c > @@ -78,7 +78,7 @@ struct callstack_filter { > > static struct lock_filter filters; > > -static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR; > +static enum lock_aggr_mode aggr_mode = LOCK_AGGR_CALLER; > > static bool needs_callstack(void) > { > @@ -1983,8 +1983,8 @@ static int __cmd_report(bool display_info) > if (select_key(false)) > goto out_delete; > > - if (show_thread_stats) > - aggr_mode = LOCK_AGGR_TASK; > + aggr_mode = show_thread_stats ? LOCK_AGGR_TASK : > + show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER; > > err = perf_session__process_events(session); > if (err) > @@ -2008,6 +2008,19 @@ static void sighandler(int sig __maybe_unused) > { > } > > +static int check_lock_report_options(const struct option *options, > + const char * const *usage) > +{ > + if (show_thread_stats && show_lock_addrs) { > + pr_err("Cannot use thread and addr mode together\n"); > + parse_options_usage(usage, options, "threads", 0); > + parse_options_usage(NULL, options, "lock-addr", 0); > + return -1; > + } > + > + return 0; > +} > + > static int check_lock_contention_options(const struct option *options, > const char * const *usage) > > @@ -2589,6 +2602,7 @@ int cmd_lock(int argc, const char **argv) > /* TODO: type */ > OPT_BOOLEAN('c', "combine-locks", &combine_locks, > "combine locks in the same class"), > + OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"), > OPT_BOOLEAN('t', "threads", &show_thread_stats, > "show per-thread lock stats"), > OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"), > @@ -2680,6 +2694,10 @@ int cmd_lock(int argc, const char **argv) > if (argc) > usage_with_options(report_usage, report_options); > } > + > + if (check_lock_report_options(report_options, report_usage) < 0) > + return -1; > + > rc = __cmd_report(false); > } else if (!strcmp(argv[0], "script")) { > /* Aliased to 'perf script' */ > -- > 2.42.0 >
On 02/11/2023 05:55, Namhyung Kim wrote: > Hello, > > On Tue, Oct 31, 2023 at 5:05 AM Nick Forrington <nick.forrington@arm.com> wrote: >> This change restores the previous default behaviour for "perf lock >> report", making the current aggregate-by-address behaviour available via >> the new "--lock-addr" command line parameter. >> >> This makes the behaviour consistent with "perf lock contention" (which >> also aggregates by caller by default, or by address when "--lock-addr" >> is specified). > I understand your concern but actually there's a difference. > "perf lock contention" is a new command which works with new > contention tracepoints whereas "perf lock report" works with old > lockdep/lockstat tracepoints which are not available in the default > configuration. > > I made "perf lock contention" compatible to "perf lock report" so > it mimics the old tracepoints behavior as much as possible using > new tracepoints. But the important difference is that new contention > tracepoints don't have lock names. The old perf lock report showed > lock names by default but contention output had to use the caller > instead. Thanks for the information. It looks like "perf lock record" requests both tracepoint types. However, on the system I used the lockdep tracepoints were not available - only lock:contention_begin/end. > >> Commit 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") >> introduced aggregation modes for "perf lock contention" and (potentially >> inadvertently) changed the behaviour of "perf lock report" from >> aggregate-by-caller to aggregate-by-address (making the prior behaviour >> inaccessible). > So it doesn't change the behavior of perf lock report. > You're adding a new (default) feature for perf lock report > to sort the output by caller. And please note that caller > info needs callstacks. perf lock record adds it by default > when it finds there are only lock contention tracepoints. > But if it really has the old tracepoints, caller won't work > unless you enabled callstack collection manually (-g). With the caveat that only lock:contention_begin/end were available in my test, there does appear to be a difference in output for perf lock report. I arrived at 688d2e8de231 via git bisect. 688d2e8de231~ (eca949b2b4ad) shows the aggregate-by-caller behaviour (seen below, and "restored" by this patch). > >> Example aggregate-by-address output: >> >> $ perf lock report -F acquired > I guess you need -l option here. This was the default output before this patch (or output with the patch and "-l"), I can clarify this if the patch goes ahead. >> Name acquired >> >> event_mutex 34 >> 21 >> 1 > This is because you used contention tracepoints > and they don't have lock names. Yes, the lockdep tracepoints weren't available for my test, although on the same machine, versions prior to 688d2e8de231 appear to show the below output rather than the above. > >> Example aggregate-by-caller output: >> >> $ perf lock report -F acquired >> Name acquired >> >> perf_trace_init+... 34 >> lock_mm_and_find... 20 >> inherit_event.co... 1 >> do_madvise+0x1f8 1 > Maybe it's ok to change the default behavior for contention > tracepoints. But when lockdep tracepoints are available > it should use the existing addr (symbol) mode. > >> Cc: stable@kernel.org >> Fixes: 688d2e8de231 ("perf lock contention: Add -l/--lock-addr option") > So, I don't think this is a fix. > > Thanks, > Namhyung Thanks, Nick > > >> Signed-off-by: Nick Forrington <nick.forrington@arm.com> >> --- >> tools/perf/Documentation/perf-lock.txt | 4 ++++ >> tools/perf/builtin-lock.c | 24 +++++++++++++++++++++--- >> 2 files changed, 25 insertions(+), 3 deletions(-) >> >> diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt >> index 503abcba1438..349333acbbfc 100644 >> --- a/tools/perf/Documentation/perf-lock.txt >> +++ b/tools/perf/Documentation/perf-lock.txt >> @@ -80,6 +80,10 @@ REPORT OPTIONS >> --combine-locks:: >> Merge lock instances in the same class (based on name). >> >> +-l:: >> +--lock-addr:: >> + Show lock contention stat by address >> + >> -t:: >> --threads:: >> The -t option is to show per-thread lock stat like below: >> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c >> index fa7419978353..3aa8ba5ad928 100644 >> --- a/tools/perf/builtin-lock.c >> +++ b/tools/perf/builtin-lock.c >> @@ -78,7 +78,7 @@ struct callstack_filter { >> >> static struct lock_filter filters; >> >> -static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR; >> +static enum lock_aggr_mode aggr_mode = LOCK_AGGR_CALLER; >> >> static bool needs_callstack(void) >> { >> @@ -1983,8 +1983,8 @@ static int __cmd_report(bool display_info) >> if (select_key(false)) >> goto out_delete; >> >> - if (show_thread_stats) >> - aggr_mode = LOCK_AGGR_TASK; >> + aggr_mode = show_thread_stats ? LOCK_AGGR_TASK : >> + show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER; >> >> err = perf_session__process_events(session); >> if (err) >> @@ -2008,6 +2008,19 @@ static void sighandler(int sig __maybe_unused) >> { >> } >> >> +static int check_lock_report_options(const struct option *options, >> + const char * const *usage) >> +{ >> + if (show_thread_stats && show_lock_addrs) { >> + pr_err("Cannot use thread and addr mode together\n"); >> + parse_options_usage(usage, options, "threads", 0); >> + parse_options_usage(NULL, options, "lock-addr", 0); >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> static int check_lock_contention_options(const struct option *options, >> const char * const *usage) >> >> @@ -2589,6 +2602,7 @@ int cmd_lock(int argc, const char **argv) >> /* TODO: type */ >> OPT_BOOLEAN('c', "combine-locks", &combine_locks, >> "combine locks in the same class"), >> + OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"), >> OPT_BOOLEAN('t', "threads", &show_thread_stats, >> "show per-thread lock stats"), >> OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"), >> @@ -2680,6 +2694,10 @@ int cmd_lock(int argc, const char **argv) >> if (argc) >> usage_with_options(report_usage, report_options); >> } >> + >> + if (check_lock_report_options(report_options, report_usage) < 0) >> + return -1; >> + >> rc = __cmd_report(false); >> } else if (!strcmp(argv[0], "script")) { >> /* Aliased to 'perf script' */ >> -- >> 2.42.0 >>
diff --git a/tools/perf/Documentation/perf-lock.txt b/tools/perf/Documentation/perf-lock.txt index 503abcba1438..349333acbbfc 100644 --- a/tools/perf/Documentation/perf-lock.txt +++ b/tools/perf/Documentation/perf-lock.txt @@ -80,6 +80,10 @@ REPORT OPTIONS --combine-locks:: Merge lock instances in the same class (based on name). +-l:: +--lock-addr:: + Show lock contention stat by address + -t:: --threads:: The -t option is to show per-thread lock stat like below: diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c index fa7419978353..3aa8ba5ad928 100644 --- a/tools/perf/builtin-lock.c +++ b/tools/perf/builtin-lock.c @@ -78,7 +78,7 @@ struct callstack_filter { static struct lock_filter filters; -static enum lock_aggr_mode aggr_mode = LOCK_AGGR_ADDR; +static enum lock_aggr_mode aggr_mode = LOCK_AGGR_CALLER; static bool needs_callstack(void) { @@ -1983,8 +1983,8 @@ static int __cmd_report(bool display_info) if (select_key(false)) goto out_delete; - if (show_thread_stats) - aggr_mode = LOCK_AGGR_TASK; + aggr_mode = show_thread_stats ? LOCK_AGGR_TASK : + show_lock_addrs ? LOCK_AGGR_ADDR : LOCK_AGGR_CALLER; err = perf_session__process_events(session); if (err) @@ -2008,6 +2008,19 @@ static void sighandler(int sig __maybe_unused) { } +static int check_lock_report_options(const struct option *options, + const char * const *usage) +{ + if (show_thread_stats && show_lock_addrs) { + pr_err("Cannot use thread and addr mode together\n"); + parse_options_usage(usage, options, "threads", 0); + parse_options_usage(NULL, options, "lock-addr", 0); + return -1; + } + + return 0; +} + static int check_lock_contention_options(const struct option *options, const char * const *usage) @@ -2589,6 +2602,7 @@ int cmd_lock(int argc, const char **argv) /* TODO: type */ OPT_BOOLEAN('c', "combine-locks", &combine_locks, "combine locks in the same class"), + OPT_BOOLEAN('l', "lock-addr", &show_lock_addrs, "show lock stats by address"), OPT_BOOLEAN('t', "threads", &show_thread_stats, "show per-thread lock stats"), OPT_INTEGER('E', "entries", &print_nr_entries, "display this many functions"), @@ -2680,6 +2694,10 @@ int cmd_lock(int argc, const char **argv) if (argc) usage_with_options(report_usage, report_options); } + + if (check_lock_report_options(report_options, report_usage) < 0) + return -1; + rc = __cmd_report(false); } else if (!strcmp(argv[0], "script")) { /* Aliased to 'perf script' */