lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ