[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fX2mr0qQhmbR2tVm_ZU6S5S2Az+Q4XfQpMAudRk+b8YOA@mail.gmail.com>
Date: Wed, 10 Dec 2025 14:06:35 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, James Clark <james.clark@...aro.org>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 4/4] perf report: Update sort key state from -F option
On Tue, Dec 9, 2025 at 6:33 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Factor out __sort_dimension__update() so that it can be called from -s
> and -F option parsing logics. Otherwise the following command cannot go
> into the annotation mode.
>
> $ perf report -F overhead,type,sym
>
> Warning: Annotation is only available for symbolic views, include "sym*" in --sort to use it.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
Reviewed-by: Ian Rogers <irogers@...gle.com>
Thanks,
Ian
> ---
> tools/perf/util/sort.c | 100 ++++++++++++++++++++++-------------------
> 1 file changed, 54 insertions(+), 46 deletions(-)
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index c51604eaae0a4b90..5e27a66e3ccb0bbe 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -3531,6 +3531,56 @@ static int add_dynamic_entry(struct evlist *evlist, const char *tok,
> return ret;
> }
>
> +static int __sort_dimension__update(struct sort_dimension *sd,
> + struct perf_hpp_list *list)
> +{
> + if (sd->entry == &sort_parent && parent_pattern) {
> + int ret = regcomp(&parent_regex, parent_pattern, REG_EXTENDED);
> + if (ret) {
> + char err[BUFSIZ];
> +
> + regerror(ret, &parent_regex, err, sizeof(err));
> + pr_err("Invalid regex: %s\n%s", parent_pattern, err);
> + return -EINVAL;
> + }
> + list->parent = 1;
> + } else if (sd->entry == &sort_sym) {
> + list->sym = 1;
> + /*
> + * perf diff displays the performance difference amongst
> + * two or more perf.data files. Those files could come
> + * from different binaries. So we should not compare
> + * their ips, but the name of symbol.
> + */
> + if (sort__mode == SORT_MODE__DIFF)
> + sd->entry->se_collapse = sort__sym_sort;
> +
> + } else if (sd->entry == &sort_sym_offset) {
> + list->sym = 1;
> + } else if (sd->entry == &sort_dso) {
> + list->dso = 1;
> + } else if (sd->entry == &sort_socket) {
> + list->socket = 1;
> + } else if (sd->entry == &sort_thread) {
> + list->thread = 1;
> + } else if (sd->entry == &sort_comm) {
> + list->comm = 1;
> + } else if (sd->entry == &sort_type_offset) {
> + symbol_conf.annotate_data_member = true;
> + } else if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to) {
> + list->sym = 1;
> + } else if (sd->entry == &sort_mem_dcacheline && cacheline_size() == 0) {
> + return -EINVAL;
> + } else if (sd->entry == &sort_mem_daddr_sym) {
> + list->sym = 1;
> + }
> +
> + if (sd->entry->se_collapse)
> + list->need_collapse = 1;
> +
> + return 0;
> +}
> +
> static int __sort_dimension__add(struct sort_dimension *sd,
> struct perf_hpp_list *list,
> int level)
> @@ -3541,8 +3591,8 @@ static int __sort_dimension__add(struct sort_dimension *sd,
> if (__sort_dimension__add_hpp_sort(sd, list, level) < 0)
> return -1;
>
> - if (sd->entry->se_collapse)
> - list->need_collapse = 1;
> + if (__sort_dimension__update(sd, list) < 0)
> + return -1;
>
> sd->taken = 1;
>
> @@ -3578,8 +3628,8 @@ static int __sort_dimension__add_output(struct perf_hpp_list *list,
> if (__sort_dimension__add_hpp_output(sd, list, level) < 0)
> return -1;
>
> - if (sd->entry->se_collapse)
> - list->need_collapse = 1;
> + if (__sort_dimension__update(sd, list) < 0)
> + return -1;
>
> sd->taken = 1;
> return 0;
> @@ -3644,39 +3694,6 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
> sort_dimension_add_dynamic_header(sd, env);
> }
>
> - if (sd->entry == &sort_parent && parent_pattern) {
> - int ret = regcomp(&parent_regex, parent_pattern, REG_EXTENDED);
> - if (ret) {
> - char err[BUFSIZ];
> -
> - regerror(ret, &parent_regex, err, sizeof(err));
> - pr_err("Invalid regex: %s\n%s", parent_pattern, err);
> - return -EINVAL;
> - }
> - list->parent = 1;
> - } else if (sd->entry == &sort_sym) {
> - list->sym = 1;
> - /*
> - * perf diff displays the performance difference amongst
> - * two or more perf.data files. Those files could come
> - * from different binaries. So we should not compare
> - * their ips, but the name of symbol.
> - */
> - if (sort__mode == SORT_MODE__DIFF)
> - sd->entry->se_collapse = sort__sym_sort;
> -
> - } else if (sd->entry == &sort_dso) {
> - list->dso = 1;
> - } else if (sd->entry == &sort_socket) {
> - list->socket = 1;
> - } else if (sd->entry == &sort_thread) {
> - list->thread = 1;
> - } else if (sd->entry == &sort_comm) {
> - list->comm = 1;
> - } else if (sd->entry == &sort_type_offset) {
> - symbol_conf.annotate_data_member = true;
> - }
> -
> return __sort_dimension__add(sd, list, level);
> }
>
> @@ -3695,9 +3712,6 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
> strlen(tok)))
> return -EINVAL;
>
> - if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
> - list->sym = 1;
> -
> __sort_dimension__add(sd, list, level);
> return 0;
> }
> @@ -3711,12 +3725,6 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
> if (sort__mode != SORT_MODE__MEMORY)
> return -EINVAL;
>
> - if (sd->entry == &sort_mem_dcacheline && cacheline_size() == 0)
> - return -EINVAL;
> -
> - if (sd->entry == &sort_mem_daddr_sym)
> - list->sym = 1;
> -
> __sort_dimension__add(sd, list, level);
> return 0;
> }
> --
> 2.52.0.223.gf5cc29aaa4-goog
>
Powered by blists - more mailing lists