linux-kernel - Re: [PATCH V2 22/23] perf tools: Allow system-wide events to keep their own CPUs

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: <CAM9d7cg8JRK2oVUvmWit=F5zhLhpqP=gD6iYBAa8_O-+c=EjPQ@mail.gmail.com>
Date:   Wed, 11 May 2022 22:27:17 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>, Ian Rogers <irogers@...gle.com>,
        Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>,
        Leo Yan <leo.yan@...aro.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 22/23] perf tools: Allow system-wide events to keep
 their own CPUs

On Fri, May 6, 2022 at 5:27 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> Currently, user_requested_cpus supplants system-wide CPUs when the evlist
> has_user_cpus. Change that so that system-wide events retain their own
> CPUs and they are added to all_cpus.
>
> Acked-by: Ian Rogers <irogers@...gle.com>
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
>  tools/lib/perf/evlist.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index 1c801f8da44f..9a6801b53274 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -40,12 +40,11 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
>          * We already have cpus for evsel (via PMU sysfs) so
>          * keep it, if there's no target cpu list defined.
>          */
> -       if (!evsel->own_cpus || evlist->has_user_cpus) {
> -               perf_cpu_map__put(evsel->cpus);
> -               evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus);
> -       } else if (!evsel->system_wide &&
> -                  !evsel->requires_cpu &&
> -                  perf_cpu_map__empty(evlist->user_requested_cpus)) {
> +       if (!evsel->own_cpus ||
> +           (!evsel->system_wide && evlist->has_user_cpus) ||
> +           (!evsel->system_wide &&
> +            !evsel->requires_cpu &&
> +            perf_cpu_map__empty(evlist->user_requested_cpus))) {

This is getting hard to understand.  IIUC this propagation basically
sets user requested cpus to evsel unless it has its own cpus, right?

But the hybrid pmus make this complex.  Maybe we can move the
logic in evlist__fix_hybrid_cpus() here and simplify it like below

if (evsel->own_cpus) {
   if (evsel->pmu->is_hybrid)
      evsel->cpus = fixup_hybrid_cpus(evsel>own_cpus,
                                      evlist->user_requested_cpus);  //?
   else
      evsel->cpus = evlist->own_cpus;  // put + get
} else {
   evsel->cpus = evlist->user_requested_cpus;  // put + get
}

Then we need to make sure evsel->pmu is set properly.

What do you think?

Thanks,
Namhyung


>                 perf_cpu_map__put(evsel->cpus);
>                 evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus);
>         } else if (evsel->cpus != evsel->own_cpus) {
> --
> 2.25.1
>