| 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
| ||
|
Message-ID: <CAP-5=fVrptOSOK+sBo0rHR1QWQ0i1WigMaFRy=So-HATKr=R9A@mail.gmail.com>
Date: Tue, 1 Oct 2024 10:53:02 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>,
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,
Ravi Bangoria <ravi.bangoria@....com>, Mark Rutland <mark.rutland@....com>,
James Clark <james.clark@....com>, Kajol Jain <kjain@...ux.ibm.com>,
Thomas Richter <tmricht@...ux.ibm.com>, Atish Patra <atishp@...shpatra.org>,
Palmer Dabbelt <palmer@...osinc.com>, Mingwei Zhang <mizhang@...gle.com>
Subject: Re: [PATCH 5/8] perf tools: Detect missing kernel features properly
On Mon, Sep 30, 2024 at 5:20 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> The evsel__detect_missing_features() is to check if the attributes of
> the evsel is supported or not. But it checks the attribute based on the
> given evsel, it might miss something if the attr doesn't have the bit or
> give incorrect results if the event is special.
>
> Also it maintains the order of the feature that was added to the kernel
> which means it can assume older features should be supported once it
> detects the current feature is working. To minimized the confusion and
> to accurately check the kernel features, I think it's better to use a
> software event and go through all the features at once.
>
> Also make the function static since it's only used in evsel.c.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/util/evsel.c | 345 +++++++++++++++++++++++++++++-----------
> tools/perf/util/evsel.h | 1 -
> 2 files changed, 249 insertions(+), 97 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index f202d28147d62a44..32e30c293d0c6198 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -20,6 +20,7 @@
> #include <linux/zalloc.h>
> #include <sys/ioctl.h>
> #include <sys/resource.h>
> +#include <sys/syscall.h>
> #include <sys/types.h>
> #include <dirent.h>
> #include <stdlib.h>
> @@ -2150,120 +2151,272 @@ int evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
> return err;
> }
>
> -bool evsel__detect_missing_features(struct evsel *evsel)
> +static bool has_attr_feature(struct perf_event_attr *attr, unsigned long flags)
> {
> + int fd = syscall(SYS_perf_event_open, attr, /*pid=*/0, /*cpu=*/-1,
> + /*group_fd=*/-1, flags);
> + close(fd);
> +
> + if (fd < 0) {
> + attr->exclude_kernel = 1;
> +
> + fd = syscall(SYS_perf_event_open, attr, /*pid=*/0, /*cpu=*/-1,
> + /*group_fd=*/-1, flags);
> + close(fd);
> + }
> +
> + if (fd < 0) {
> + attr->exclude_hv = 1;
> +
> + fd = syscall(SYS_perf_event_open, attr, /*pid=*/0, /*cpu=*/-1,
> + /*group_fd=*/-1, flags);
> + close(fd);
> + }
> +
> + if (fd < 0) {
> + attr->exclude_guest = 1;
> +
> + fd = syscall(SYS_perf_event_open, attr, /*pid=*/0, /*cpu=*/-1,
> + /*group_fd=*/-1, flags);
> + close(fd);
> + }
> +
> + attr->exclude_kernel = 0;
> + attr->exclude_guest = 0;
> + attr->exclude_hv = 0;
> +
> + return fd >= 0;
> +}
> +
> +static void evsel__detect_missing_brstack_features(struct evsel *evsel)
In the future could other PMU specific unsupported features be added
not just brstack? Perhaps evsel__detect_missing_pmu_features would
better capture that.
> +{
> + static bool detection_done = false;
> + struct perf_event_attr attr = {
> + .type = evsel->core.attr.type,
> + .config = evsel->core.attr.config,
> + .disabled = 1,
> + .sample_type = PERF_SAMPLE_BRANCH_STACK,
> + .sample_period = 1000,
> + };
> + int old_errno;
> +
> + if (detection_done)
> + return;
> +
> + old_errno = errno;
> +
> /*
> * Must probe features in the order they were added to the
> - * perf_event_attr interface.
> + * perf_event_attr interface. These are PMU specific limitation
> + * so we can detect with the given hardware event and stop on the
> + * first one succeeded.
> */
> - if (!perf_missing_features.branch_counters &&
> - (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS)) {
> - perf_missing_features.branch_counters = true;
> - pr_debug2("switching off branch counters support\n");
> - return true;
> - } else if (!perf_missing_features.read_lost &&
> - (evsel->core.attr.read_format & PERF_FORMAT_LOST)) {
> - perf_missing_features.read_lost = true;
> - pr_debug2("switching off PERF_FORMAT_LOST support\n");
> +
> + /* Please add new feature detection here. */
> +
> + attr.branch_sample_type = PERF_SAMPLE_BRANCH_COUNTERS;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.branch_counters = true;
It feels like these global variables should be part of the PMU state.
There is already perf_pmu.missing_features.
Thanks,
Ian
> + pr_debug2("switching off branch counters support\n");
> +
> + attr.branch_sample_type = PERF_SAMPLE_BRANCH_HW_INDEX;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.branch_hw_idx = true;
> + pr_debug2("switching off branch HW index support\n");
> +
> + attr.branch_sample_type = PERF_SAMPLE_BRANCH_NO_CYCLES | PERF_SAMPLE_BRANCH_NO_FLAGS;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.lbr_flags = true;
> + pr_debug2_peo("switching off branch sample type no (cycles/flags)\n");
> +
> +found:
> + detection_done = true;
> + errno = old_errno;
> +}
> +
> +static bool evsel__detect_missing_features(struct evsel *evsel)
> +{
> + static bool detection_done = false;
> + struct perf_event_attr attr = {
> + .type = PERF_TYPE_SOFTWARE,
> + .config = PERF_COUNT_SW_TASK_CLOCK,
> + .disabled = 1,
> + };
> + int old_errno;
> +
> + if (evsel__has_br_stack(evsel))
> + evsel__detect_missing_brstack_features(evsel);
> +
> + if (detection_done)
> + goto check;
> +
> + old_errno = errno;
> +
> + /*
> + * Must probe features in the order they were added to the
> + * perf_event_attr interface. These are kernel core limitation
> + * not PMU-specific so we can detect with a software event and
> + * stop on the first one succeeded.
> + */
> +
> + /* Please add new feature detection here. */
> +
> + attr.read_format = PERF_FORMAT_LOST;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.read_lost = true;
> + pr_debug2("switching off PERF_FORMAT_LOST support\n");
> + attr.read_format = 0;
> +
> + attr.sample_type = PERF_SAMPLE_WEIGHT_STRUCT;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.weight_struct = true;
> + pr_debug2("switching off weight struct support\n");
> + attr.sample_type = 0;
> +
> + attr.sample_type = PERF_SAMPLE_CODE_PAGE_SIZE;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.code_page_size = true;
> + pr_debug2_peo("Kernel has no PERF_SAMPLE_CODE_PAGE_SIZE support\n");
> + attr.sample_type = 0;
> +
> + attr.sample_type = PERF_SAMPLE_DATA_PAGE_SIZE;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.data_page_size = true;
> + pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support\n");
> + attr.sample_type = 0;
> +
> + attr.cgroup = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.cgroup = true;
> + pr_debug2_peo("Kernel has no cgroup sampling support\n");
> + attr.cgroup = 0;
> +
> + attr.aux_output = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.aux_output = true;
> + pr_debug2_peo("Kernel has no attr.aux_output support\n");
> + attr.aux_output = 0;
> +
> + attr.bpf_event = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.bpf = true;
> + pr_debug2_peo("switching off bpf_event\n");
> + attr.bpf_event = 0;
> +
> + attr.ksymbol = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.ksymbol = true;
> + pr_debug2_peo("switching off ksymbol\n");
> + attr.ksymbol = 0;
> +
> + attr.write_backward = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.write_backward = true;
> + pr_debug2_peo("switching off write_backward\n");
> + attr.write_backward = 0;
> +
> + attr.use_clockid = 1;
> + attr.clockid = CLOCK_MONOTONIC;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.clockid = true;
> + pr_debug2_peo("switching off clockid\n");
> + attr.use_clockid = 0;
> + attr.clockid = 0;
> +
> + if (has_attr_feature(&attr, /*flags=*/PERF_FLAG_FD_CLOEXEC))
> + goto found;
> + perf_missing_features.cloexec = true;
> + pr_debug2_peo("switching off cloexec flag\n");
> +
> + attr.mmap2 = 1;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.mmap2 = true;
> + pr_debug2_peo("switching off mmap2\n");
> + attr.mmap2 = 0;
> +
> + /* set this unconditionally? */
> + perf_missing_features.sample_id_all = true;
> + pr_debug2_peo("switching off sample_id_all\n");
> +
> + attr.inherit = 1;
> + attr.read_format = PERF_FORMAT_GROUP;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.group_read = true;
> + pr_debug2_peo("switching off group read\n");
> + attr.inherit = 0;
> + attr.read_format = 0;
> +
> +found:
> + detection_done = true;
> + errno = old_errno;
> +
> +check:
> + if ((evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) &&
> + perf_missing_features.branch_counters)
> return true;
> - } else if (!perf_missing_features.weight_struct &&
> - (evsel->core.attr.sample_type & PERF_SAMPLE_WEIGHT_STRUCT)) {
> - perf_missing_features.weight_struct = true;
> - pr_debug2("switching off weight struct support\n");
> +
> + if ((evsel->core.attr.read_format & PERF_FORMAT_LOST) &&
> + perf_missing_features.read_lost)
> return true;
> - } else if (!perf_missing_features.code_page_size &&
> - (evsel->core.attr.sample_type & PERF_SAMPLE_CODE_PAGE_SIZE)) {
> - perf_missing_features.code_page_size = true;
> - pr_debug2_peo("Kernel has no PERF_SAMPLE_CODE_PAGE_SIZE support, bailing out\n");
> - return false;
> - } else if (!perf_missing_features.data_page_size &&
> - (evsel->core.attr.sample_type & PERF_SAMPLE_DATA_PAGE_SIZE)) {
> - perf_missing_features.data_page_size = true;
> - pr_debug2_peo("Kernel has no PERF_SAMPLE_DATA_PAGE_SIZE support, bailing out\n");
> - return false;
> - } else if (!perf_missing_features.cgroup && evsel->core.attr.cgroup) {
> - perf_missing_features.cgroup = true;
> - pr_debug2_peo("Kernel has no cgroup sampling support, bailing out\n");
> - return false;
> - } else if (!perf_missing_features.branch_hw_idx &&
> - (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_HW_INDEX)) {
> - perf_missing_features.branch_hw_idx = true;
> - pr_debug2("switching off branch HW index support\n");
> +
> + if ((evsel->core.attr.sample_type & PERF_SAMPLE_WEIGHT_STRUCT) &&
> + perf_missing_features.weight_struct)
> return true;
> - } else if (!perf_missing_features.aux_output && evsel->core.attr.aux_output) {
> - perf_missing_features.aux_output = true;
> - pr_debug2_peo("Kernel has no attr.aux_output support, bailing out\n");
> - return false;
> - } else if (!perf_missing_features.bpf && evsel->core.attr.bpf_event) {
> - perf_missing_features.bpf = true;
> - pr_debug2_peo("switching off bpf_event\n");
> +
> + if (evsel->core.attr.use_clockid && evsel->core.attr.clockid != CLOCK_MONOTONIC &&
> + !perf_missing_features.clockid) {
> + perf_missing_features.clockid_wrong = true;
> return true;
> - } else if (!perf_missing_features.ksymbol && evsel->core.attr.ksymbol) {
> - perf_missing_features.ksymbol = true;
> - pr_debug2_peo("switching off ksymbol\n");
> + }
> +
> + if (evsel->core.attr.use_clockid && perf_missing_features.clockid)
> return true;
> - } else if (!perf_missing_features.write_backward && evsel->core.attr.write_backward) {
> - perf_missing_features.write_backward = true;
> - pr_debug2_peo("switching off write_backward\n");
> - return false;
> - } else if (!perf_missing_features.clockid_wrong && evsel->core.attr.use_clockid) {
> - perf_missing_features.clockid_wrong = true;
> - pr_debug2_peo("switching off clockid\n");
> +
> + if ((evsel->open_flags & PERF_FLAG_FD_CLOEXEC) &&
> + perf_missing_features.cloexec)
> return true;
> - } else if (!perf_missing_features.clockid && evsel->core.attr.use_clockid) {
> - perf_missing_features.clockid = true;
> - pr_debug2_peo("switching off use_clockid\n");
> +
> + if (evsel->core.attr.mmap2 && perf_missing_features.mmap2)
> return true;
> - } else if (!perf_missing_features.cloexec && (evsel->open_flags & PERF_FLAG_FD_CLOEXEC)) {
> - perf_missing_features.cloexec = true;
> - pr_debug2_peo("switching off cloexec flag\n");
> +
> + if ((evsel->core.attr.branch_sample_type & (PERF_SAMPLE_BRANCH_NO_FLAGS |
> + PERF_SAMPLE_BRANCH_NO_CYCLES)) &&
> + perf_missing_features.lbr_flags)
> return true;
> - } else if (!perf_missing_features.mmap2 && evsel->core.attr.mmap2) {
> - perf_missing_features.mmap2 = true;
> - pr_debug2_peo("switching off mmap2\n");
> +
> + if (evsel->core.attr.inherit && (evsel->core.attr.read_format & PERF_FORMAT_GROUP) &&
> + perf_missing_features.group_read)
> return true;
> - } else if (evsel->core.attr.exclude_guest || evsel->core.attr.exclude_host) {
> - if (evsel->pmu == NULL)
> - evsel->pmu = evsel__find_pmu(evsel);
> -
> - if (evsel->pmu)
> - evsel->pmu->missing_features.exclude_guest = true;
> - else {
> - /* we cannot find PMU, disable attrs now */
> - evsel->core.attr.exclude_host = false;
> - evsel->core.attr.exclude_guest = false;
> - }
>
> - if (evsel->exclude_GH) {
> - pr_debug2_peo("PMU has no exclude_host/guest support, bailing out\n");
> - return false;
> - }
> - if (!perf_missing_features.exclude_guest) {
> - perf_missing_features.exclude_guest = true;
> - pr_debug2_peo("switching off exclude_guest, exclude_host\n");
> - }
> + if (evsel->core.attr.ksymbol && perf_missing_features.ksymbol)
> return true;
> - } else if (!perf_missing_features.sample_id_all) {
> - perf_missing_features.sample_id_all = true;
> - pr_debug2_peo("switching off sample_id_all\n");
> +
> + if (evsel->core.attr.bpf_event && perf_missing_features.bpf)
> return true;
> - } else if (!perf_missing_features.lbr_flags &&
> - (evsel->core.attr.branch_sample_type &
> - (PERF_SAMPLE_BRANCH_NO_CYCLES |
> - PERF_SAMPLE_BRANCH_NO_FLAGS))) {
> - perf_missing_features.lbr_flags = true;
> - pr_debug2_peo("switching off branch sample type no (cycles/flags)\n");
> +
> + if ((evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_HW_INDEX) &&
> + perf_missing_features.branch_hw_idx)
> return true;
> - } else if (!perf_missing_features.group_read &&
> - evsel->core.attr.inherit &&
> - (evsel->core.attr.read_format & PERF_FORMAT_GROUP) &&
> - evsel__is_group_leader(evsel)) {
> - perf_missing_features.group_read = true;
> - pr_debug2_peo("switching off group read\n");
> +
> + if (evsel->core.attr.sample_id_all && perf_missing_features.sample_id_all)
> return true;
> - } else {
> - return false;
> - }
> +
> + return false;
> }
>
> static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 3e751ea769ac4d3a..ea3140cd91c589fd 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -368,7 +368,6 @@ int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
> void evsel__close(struct evsel *evsel);
> int evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
> struct perf_thread_map *threads);
> -bool evsel__detect_missing_features(struct evsel *evsel);
>
> bool evsel__precise_ip_fallback(struct evsel *evsel);
>
> --
> 2.46.1.824.gd892dcdcdd-goog
>
Powered by blists - more mailing lists