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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fVDjOfM8rZStYJCFPcTD3uhJDNSf_=JrkwYjFrnoOLC=w@mail.gmail.com>
Date: Thu, 1 Feb 2024 06:24:51 -0800
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@....com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Namhyung Kim <namhyung@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, 
	Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach <mike.leach@...aro.org>, 
	Leo Yan <leo.yan@...aro.org>, John Garry <john.g.garry@...cle.com>, 
	Will Deacon <will@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Darren Hart <dvhart@...radead.org>, 
	Davidlohr Bueso <dave@...olabs.net>, André Almeida <andrealmeid@...lia.com>, 
	Kan Liang <kan.liang@...ux.intel.com>, K Prateek Nayak <kprateek.nayak@....com>, 
	Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, 
	Kajol Jain <kjain@...ux.ibm.com>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, 
	Andrew Jones <ajones@...tanamicro.com>, Alexandre Ghiti <alexghiti@...osinc.com>, 
	Atish Patra <atishp@...osinc.com>, "Steinar H. Gunderson" <sesse@...gle.com>, 
	Yang Jihong <yangjihong1@...wei.com>, Yang Li <yang.lee@...ux.alibaba.com>, 
	Changbin Du <changbin.du@...wei.com>, Sandipan Das <sandipan.das@....com>, 
	Ravi Bangoria <ravi.bangoria@....com>, Paran Lee <p4ranlee@...il.com>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Huacai Chen <chenhuacai@...nel.org>, 
	Yanteng Si <siyanteng@...ngson.cn>, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org, coresight@...ts.linaro.org, 
	linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org
Subject: Re: [PATCH v2 3/8] perf arm-spe/cs-etm: Directly iterate CPU maps

On Thu, Feb 1, 2024 at 3:25 AM James Clark <james.clark@....com> wrote:
>
>
>
> On 01/02/2024 04:22, Ian Rogers wrote:
> > Rather than iterate all CPUs and see if they are in CPU maps, directly
> > iterate the CPU map. Similarly make use of the intersect
> > function. Switch perf_cpu_map__has_any_cpu_or_is_empty to more
> > appropriate alternatives.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/perf/arch/arm/util/cs-etm.c    | 77 ++++++++++++----------------
> >  tools/perf/arch/arm64/util/arm-spe.c |  4 +-
> >  2 files changed, 34 insertions(+), 47 deletions(-)
> >
> > diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> > index 77e6663c1703..f4378ba0b8d6 100644
> > --- a/tools/perf/arch/arm/util/cs-etm.c
> > +++ b/tools/perf/arch/arm/util/cs-etm.c
> > @@ -197,38 +197,32 @@ static int cs_etm_validate_timestamp(struct auxtrace_record *itr,
> >  static int cs_etm_validate_config(struct auxtrace_record *itr,
> >                                 struct evsel *evsel)
> >  {
> > -     int i, err = -EINVAL;
> > +     int idx, err = -EINVAL;
> >       struct perf_cpu_map *event_cpus = evsel->evlist->core.user_requested_cpus;
> >       struct perf_cpu_map *online_cpus = perf_cpu_map__new_online_cpus();
> > +     struct perf_cpu_map *intersect_cpus = perf_cpu_map__intersect(event_cpus, online_cpus);
>
> Hi Ian,
>
> This has the same issue as V1. 'any' intersect 'online' == empty. Now no
> validation happens anymore. For this to be the same as it used to be,
> validation has to happen on _all_ cores when event_cpus == -1. So it
> needs to be 'any' intersect 'online' == 'online'.
>
> Same issue below with cs_etm_info_priv_size()

Thanks James, sorry for the churn. I'll work on that for v3.

Ian

> Thanks
> James
>
> > +     struct perf_cpu cpu;
> >
> > -     /* Set option of each CPU we have */
> > -     for (i = 0; i < cpu__max_cpu().cpu; i++) {
> > -             struct perf_cpu cpu = { .cpu = i, };
> > -
> > -             /*
> > -              * In per-cpu case, do the validation for CPUs to work with.
> > -              * In per-thread case, the CPU map is empty.  Since the traced
> > -              * program can run on any CPUs in this case, thus don't skip
> > -              * validation.
> > -              */
> > -             if (!perf_cpu_map__has_any_cpu_or_is_empty(event_cpus) &&
> > -                 !perf_cpu_map__has(event_cpus, cpu))
> > -                     continue;
> > -
> > -             if (!perf_cpu_map__has(online_cpus, cpu))
> > -                     continue;
> > +     perf_cpu_map__put(online_cpus);
> >
> > -             err = cs_etm_validate_context_id(itr, evsel, i);
> > +     /*
> > +      * Set option of each CPU we have. In per-cpu case, do the validation
> > +      * for CPUs to work with.  In per-thread case, the CPU map is empty.
> > +      * Since the traced program can run on any CPUs in this case, thus don't
> > +      * skip validation.
> > +      */
> > +     perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
> > +             err = cs_etm_validate_context_id(itr, evsel, cpu.cpu);
> >               if (err)
> >                       goto out;
> > -             err = cs_etm_validate_timestamp(itr, evsel, i);
> > +             err = cs_etm_validate_timestamp(itr, evsel, cpu.cpu);
> >               if (err)
> >                       goto out;
> >       }
> >
> >       err = 0;
> >  out:
> > -     perf_cpu_map__put(online_cpus);
> > +     perf_cpu_map__put(intersect_cpus);
> >       return err;
> >  }
> >
> > @@ -435,7 +429,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
> >        * Also the case of per-cpu mmaps, need the contextID in order to be notified
> >        * when a context switch happened.
> >        */
> > -     if (!perf_cpu_map__has_any_cpu_or_is_empty(cpus)) {
> > +     if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
> >               evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
> >                                          "timestamp", 1);
> >               evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel,
> > @@ -461,7 +455,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
> >       evsel->core.attr.sample_period = 1;
> >
> >       /* In per-cpu case, always need the time of mmap events etc */
> > -     if (!perf_cpu_map__has_any_cpu_or_is_empty(cpus))
> > +     if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus))
> >               evsel__set_sample_bit(evsel, TIME);
> >
> >       err = cs_etm_validate_config(itr, cs_etm_evsel);
> > @@ -533,38 +527,32 @@ static size_t
> >  cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
> >                     struct evlist *evlist __maybe_unused)
> >  {
> > -     int i;
> > +     int idx;
> >       int etmv3 = 0, etmv4 = 0, ete = 0;
> >       struct perf_cpu_map *event_cpus = evlist->core.user_requested_cpus;
> >       struct perf_cpu_map *online_cpus = perf_cpu_map__new_online_cpus();
> > +     struct perf_cpu cpu;
> >
> >       /* cpu map is not empty, we have specific CPUs to work with */
> > -     if (!perf_cpu_map__has_any_cpu_or_is_empty(event_cpus)) {
> > -             for (i = 0; i < cpu__max_cpu().cpu; i++) {
> > -                     struct perf_cpu cpu = { .cpu = i, };
> > -
> > -                     if (!perf_cpu_map__has(event_cpus, cpu) ||
> > -                         !perf_cpu_map__has(online_cpus, cpu))
> > -                             continue;
> > +     if (!perf_cpu_map__is_empty(event_cpus)) {
> > +             struct perf_cpu_map *intersect_cpus =
> > +                     perf_cpu_map__intersect(event_cpus, online_cpus);
> >
> > -                     if (cs_etm_is_ete(itr, i))
> > +             perf_cpu_map__for_each_cpu_skip_any(cpu, idx, intersect_cpus) {
> > +                     if (cs_etm_is_ete(itr, cpu.cpu))
> >                               ete++;
> > -                     else if (cs_etm_is_etmv4(itr, i))
> > +                     else if (cs_etm_is_etmv4(itr, cpu.cpu))
> >                               etmv4++;
> >                       else
> >                               etmv3++;
> >               }
> > +             perf_cpu_map__put(intersect_cpus);
> >       } else {
> >               /* get configuration for all CPUs in the system */
> > -             for (i = 0; i < cpu__max_cpu().cpu; i++) {
> > -                     struct perf_cpu cpu = { .cpu = i, };
> > -
> > -                     if (!perf_cpu_map__has(online_cpus, cpu))
> > -                             continue;
> > -
> > -                     if (cs_etm_is_ete(itr, i))
> > +             perf_cpu_map__for_each_cpu(cpu, idx, online_cpus) {
> > +                     if (cs_etm_is_ete(itr, cpu.cpu))
> >                               ete++;
> > -                     else if (cs_etm_is_etmv4(itr, i))
> > +                     else if (cs_etm_is_etmv4(itr, cpu.cpu))
> >                               etmv4++;
> >                       else
> >                               etmv3++;
> > @@ -814,15 +802,14 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,
> >               return -EINVAL;
> >
> >       /* If the cpu_map is empty all online CPUs are involved */
> > -     if (perf_cpu_map__has_any_cpu_or_is_empty(event_cpus)) {
> > +     if (perf_cpu_map__is_empty(event_cpus)) {
> >               cpu_map = online_cpus;
> >       } else {
> >               /* Make sure all specified CPUs are online */
> > -             for (i = 0; i < perf_cpu_map__nr(event_cpus); i++) {
> > -                     struct perf_cpu cpu = { .cpu = i, };
> > +             struct perf_cpu cpu;
> >
> > -                     if (perf_cpu_map__has(event_cpus, cpu) &&
> > -                         !perf_cpu_map__has(online_cpus, cpu))
> > +             perf_cpu_map__for_each_cpu(cpu, i, event_cpus) {
> > +                     if (!perf_cpu_map__has(online_cpus, cpu))
> >                               return -EINVAL;
> >               }
> >
> > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> > index 51ccbfd3d246..0b52e67edb3b 100644
> > --- a/tools/perf/arch/arm64/util/arm-spe.c
> > +++ b/tools/perf/arch/arm64/util/arm-spe.c
> > @@ -232,7 +232,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
> >        * In the case of per-cpu mmaps, sample CPU for AUX event;
> >        * also enable the timestamp tracing for samples correlation.
> >        */
> > -     if (!perf_cpu_map__has_any_cpu_or_is_empty(cpus)) {
> > +     if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
> >               evsel__set_sample_bit(arm_spe_evsel, CPU);
> >               evsel__set_config_if_unset(arm_spe_pmu, arm_spe_evsel,
> >                                          "ts_enable", 1);
> > @@ -265,7 +265,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
> >       tracking_evsel->core.attr.sample_period = 1;
> >
> >       /* In per-cpu case, always need the time of mmap events etc */
> > -     if (!perf_cpu_map__has_any_cpu_or_is_empty(cpus)) {
> > +     if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) {
> >               evsel__set_sample_bit(tracking_evsel, TIME);
> >               evsel__set_sample_bit(tracking_evsel, CPU);
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ