| 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
| ||
|
Date: Wed, 30 Oct 2019 11:06:00 +0100
From: Jiri Olsa <jolsa@...hat.com>
To: Andi Kleen <andi@...stfloor.org>
Cc: acme@...nel.org, jolsa@...nel.org, eranian@...gle.com,
linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH v3 5/7] perf stat: Use affinity for opening events
On Fri, Oct 25, 2019 at 11:14:15AM -0700, Andi Kleen wrote:
SNIP
>
> +enum counter_recovery {
> + COUNTER_SKIP,
> + COUNTER_RETRY,
> + COUNTER_FATAL,
> +};
> +
> +static enum counter_recovery stat_handle_error(struct evsel *counter)
> +{
> + char msg[BUFSIZ];
> + /*
> + * PPC returns ENXIO for HW counters until 2.6.37
> + * (behavior changed with commit b0a873e).
> + */
> + if (errno == EINVAL || errno == ENOSYS ||
> + errno == ENOENT || errno == EOPNOTSUPP ||
> + errno == ENXIO) {
> + if (verbose > 0)
> + ui__warning("%s event is not supported by the kernel.\n",
> + perf_evsel__name(counter));
> + counter->supported = false;
> + counter->errored = true;
> +
> + if ((counter->leader != counter) ||
> + !(counter->leader->core.nr_members > 1))
> + return COUNTER_SKIP;
> + } else if (perf_evsel__fallback(counter, errno, msg, sizeof(msg))) {
> + if (verbose > 0)
> + ui__warning("%s\n", msg);
> + return COUNTER_RETRY;
> + } else if (target__has_per_thread(&target) &&
> + evsel_list->core.threads &&
> + evsel_list->core.threads->err_thread != -1) {
> + /*
> + * For global --per-thread case, skip current
> + * error thread.
> + */
> + if (!thread_map__remove(evsel_list->core.threads,
> + evsel_list->core.threads->err_thread)) {
> + evsel_list->core.threads->err_thread = -1;
> + return COUNTER_RETRY;
> + }
> + }
> +
> + perf_evsel__open_strerror(counter, &target,
> + errno, msg, sizeof(msg));
> + ui__error("%s\n", msg);
> +
> + if (child_pid != -1)
> + kill(child_pid, SIGTERM);
> + return COUNTER_FATAL;
> +}
there's lot of code movement and other factoring together with
affinity changes, please separate those into separate patches
thanks,
jirka
> +
> static int __run_perf_stat(int argc, const char **argv, int run_idx)
> {
> int interval = stat_config.interval;
> @@ -428,11 +481,15 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> char msg[BUFSIZ];
> unsigned long long t0, t1;
> struct evsel *counter;
> + struct perf_cpu_map *cpus;
> struct timespec ts;
> size_t l;
> int status = 0;
> const bool forks = (argc > 0);
> bool is_pipe = STAT_RECORD ? perf_stat.data.is_pipe : false;
> + struct affinity affinity;
> + int i, cpu;
> + bool second_pass = false;
>
> if (interval) {
> ts.tv_sec = interval / USEC_PER_MSEC;
> @@ -457,61 +514,109 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> if (group)
> perf_evlist__set_leader(evsel_list);
>
> - evlist__for_each_entry(evsel_list, counter) {
> + if (affinity__setup(&affinity) < 0)
> + return -1;
> +
> + cpus = evlist__cpu_iter_start(evsel_list);
> + cpumap__for_each_cpu (cpus, i, cpu) {
> + affinity__set(&affinity, cpu);
> +
> + evlist__for_each_entry(evsel_list, counter) {
> + if (evlist__cpu_iter_skip(counter, cpu))
> + continue;
> + if (counter->reset_group || counter->errored)
> + continue;
> + evlist__cpu_iter_next(counter);
> try_again:
> - if (create_perf_stat_counter(counter, &stat_config, &target) < 0) {
> -
> - /* Weak group failed. Reset the group. */
> - if ((errno == EINVAL || errno == EBADF) &&
> - counter->leader != counter &&
> - counter->weak_group) {
> - counter = perf_evlist__reset_weak_group(evsel_list, counter);
> - goto try_again;
> - }
> + if (create_perf_stat_counter(counter, &stat_config, &target,
> + counter->cpu_index - 1) < 0) {
>
> - /*
> - * PPC returns ENXIO for HW counters until 2.6.37
> - * (behavior changed with commit b0a873e).
> - */
> - if (errno == EINVAL || errno == ENOSYS ||
> - errno == ENOENT || errno == EOPNOTSUPP ||
> - errno == ENXIO) {
> - if (verbose > 0)
> - ui__warning("%s event is not supported by the kernel.\n",
> - perf_evsel__name(counter));
> - counter->supported = false;
> -
> - if ((counter->leader != counter) ||
> - !(counter->leader->core.nr_members > 1))
> - continue;
> - } else if (perf_evsel__fallback(counter, errno, msg, sizeof(msg))) {
> - if (verbose > 0)
> - ui__warning("%s\n", msg);
> - goto try_again;
> - } else if (target__has_per_thread(&target) &&
> - evsel_list->core.threads &&
> - evsel_list->core.threads->err_thread != -1) {
> /*
> - * For global --per-thread case, skip current
> - * error thread.
> + * Weak group failed. We cannot just undo this here
> + * because earlier CPUs might be in group mode, and the kernel
> + * doesn't support mixing group and non group reads. Defer
> + * it to later.
> + * Don't close here because we're in the wrong affinity.
> */
> - if (!thread_map__remove(evsel_list->core.threads,
> - evsel_list->core.threads->err_thread)) {
> - evsel_list->core.threads->err_thread = -1;
> + if ((errno == EINVAL || errno == EBADF) &&
> + counter->leader != counter &&
> + counter->weak_group) {
> + perf_evlist__reset_weak_group(evsel_list, counter, false);
> + assert(counter->reset_group);
> + second_pass = true;
> + continue;
> + }
> +
> + switch (stat_handle_error(counter)) {
> + case COUNTER_FATAL:
> + return -1;
> + case COUNTER_RETRY:
> goto try_again;
> + case COUNTER_SKIP:
> + continue;
> + default:
> + break;
> }
> +
> }
> + counter->supported = true;
> + }
> + }
>
> - perf_evsel__open_strerror(counter, &target,
> - errno, msg, sizeof(msg));
> - ui__error("%s\n", msg);
> + if (second_pass) {
> + /*
> + * Now redo all the weak group after closing them,
> + * and also close errored counters.
> + */
>
> - if (child_pid != -1)
> - kill(child_pid, SIGTERM);
> + cpus = evlist__cpu_iter_start(evsel_list);
> + cpumap__for_each_cpu (cpus, i, cpu) {
> + affinity__set(&affinity, cpu);
> + /* First close errored or weak retry */
> + evlist__for_each_entry(evsel_list, counter) {
> + if (!counter->reset_group && !counter->errored)
> + continue;
> + if (evlist__cpu_iter_skip(counter, cpu))
> + continue;
> + perf_evsel__close_cpu(&counter->core, counter->cpu_index);
> + }
> + /* Now reopen weak */
> + evlist__for_each_entry(evsel_list, counter) {
> + if (!counter->reset_group)
> + continue;
> + if (evlist__cpu_iter_skip(counter, cpu))
> + continue;
> + evlist__cpu_iter_next(counter);
> +try_again_reset:
> + pr_debug2("reopening weak %s\n", perf_evsel__name(counter));
> + if (create_perf_stat_counter(counter, &stat_config, &target,
> + counter->cpu_index - 1) < 0) {
> +
> + switch (stat_handle_error(counter)) {
> + case COUNTER_FATAL:
> + return -1;
> + case COUNTER_RETRY:
> + goto try_again_reset;
> + case COUNTER_SKIP:
> + continue;
> + default:
> + break;
> + }
> + }
> + counter->supported = true;
> + }
> + }
> + }
> + affinity__cleanup(&affinity);
>
> - return -1;
> + evlist__for_each_entry(evsel_list, counter) {
> + if (!counter->supported) {
> + perf_evsel__free_fd(&counter->core);
> + continue;
> }
> - counter->supported = true;
> + /* Must have consumed all map indexes */
> + assert(!counter->errored &&
> + counter->cpu_index == counter->core.cpus->nr);
>
> l = strlen(counter->unit);
> if (l > stat_config.unit_width)
> diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
> index 1ee8704e2284..1e8a9f5c356d 100644
> --- a/tools/perf/tests/event-times.c
> +++ b/tools/perf/tests/event-times.c
> @@ -125,7 +125,7 @@ static int attach__cpu_disabled(struct evlist *evlist)
>
> evsel->core.attr.disabled = 1;
>
> - err = perf_evsel__open_per_cpu(evsel, cpus);
> + err = perf_evsel__open_per_cpu(evsel, cpus, -1);
> if (err) {
> if (err == -EACCES)
> return TEST_SKIP;
> @@ -152,7 +152,7 @@ static int attach__cpu_enabled(struct evlist *evlist)
> return -1;
> }
>
> - err = perf_evsel__open_per_cpu(evsel, cpus);
> + err = perf_evsel__open_per_cpu(evsel, cpus, -1);
> if (err == -EACCES)
> return TEST_SKIP;
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index aeb82de36043..ca9b06979fc0 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1635,7 +1635,8 @@ void perf_evlist__force_leader(struct evlist *evlist)
> }
>
> struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
> - struct evsel *evsel)
> + struct evsel *evsel,
> + bool close)
> {
> struct evsel *c2, *leader;
> bool is_open = true;
> @@ -1652,10 +1653,11 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
> if (c2 == evsel)
> is_open = false;
> if (c2->leader == leader) {
> - if (is_open)
> + if (is_open && close)
> perf_evsel__close(&c2->core);
> c2->leader = c2;
> c2->core.nr_members = 0;
> + c2->reset_group = true;
> }
> }
> return leader;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index c1deb8ebdcea..d9174d565db3 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -351,5 +351,6 @@ bool perf_evlist__exclude_kernel(struct evlist *evlist);
> void perf_evlist__force_leader(struct evlist *evlist);
>
> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> - struct evsel *evsel);
> + struct evsel *evsel,
> + bool close);
> #endif /* __PERF_EVLIST_H */
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d4451846af93..7106f9a067df 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1569,8 +1569,9 @@ static int perf_event_open(struct evsel *evsel,
> return fd;
> }
>
> -int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
> - struct perf_thread_map *threads)
> +static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
> + struct perf_thread_map *threads,
> + int start_cpu, int end_cpu)
> {
> int cpu, thread, nthreads;
> unsigned long flags = PERF_FLAG_FD_CLOEXEC;
> @@ -1647,7 +1648,7 @@ int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
>
> display_attr(&evsel->core.attr);
>
> - for (cpu = 0; cpu < cpus->nr; cpu++) {
> + for (cpu = start_cpu; cpu < end_cpu; cpu++) {
>
> for (thread = 0; thread < nthreads; thread++) {
> int fd, group_fd;
> @@ -1825,6 +1826,12 @@ int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
> return err;
> }
>
> +int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
> + struct perf_thread_map *threads)
> +{
> + return evsel__open_cpu(evsel, cpus, threads, 0, cpus ? cpus->nr : 1);
> +}
> +
> void evsel__close(struct evsel *evsel)
> {
> perf_evsel__close(&evsel->core);
> @@ -1832,9 +1839,10 @@ void evsel__close(struct evsel *evsel)
> }
>
> int perf_evsel__open_per_cpu(struct evsel *evsel,
> - struct perf_cpu_map *cpus)
> + struct perf_cpu_map *cpus,
> + int cpu)
> {
> - return evsel__open(evsel, cpus, NULL);
> + return evsel__open_cpu(evsel, cpus, NULL, cpu, cpu + 1);
> }
>
> int perf_evsel__open_per_thread(struct evsel *evsel,
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 2e3b011ed09e..d5440a928745 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -94,6 +94,8 @@ struct evsel {
> struct evsel *metric_leader;
> bool collect_stat;
> bool weak_group;
> + bool reset_group;
> + bool errored;
> bool percore;
> int cpu_index;
> const char *pmu_name;
> @@ -223,7 +225,8 @@ int evsel__enable(struct evsel *evsel);
> int evsel__disable(struct evsel *evsel);
>
> int perf_evsel__open_per_cpu(struct evsel *evsel,
> - struct perf_cpu_map *cpus);
> + struct perf_cpu_map *cpus,
> + int cpu);
> int perf_evsel__open_per_thread(struct evsel *evsel,
> struct perf_thread_map *threads);
> int evsel__open(struct evsel *evsel, struct perf_cpu_map *cpus,
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 6822e4ffe224..3aebe732e886 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -463,7 +463,8 @@ size_t perf_event__fprintf_stat_config(union perf_event *event, FILE *fp)
>
> int create_perf_stat_counter(struct evsel *evsel,
> struct perf_stat_config *config,
> - struct target *target)
> + struct target *target,
> + int cpu)
> {
> struct perf_event_attr *attr = &evsel->core.attr;
> struct evsel *leader = evsel->leader;
> @@ -517,7 +518,7 @@ int create_perf_stat_counter(struct evsel *evsel,
> }
>
> if (target__has_cpu(target) && !target__has_per_thread(target))
> - return perf_evsel__open_per_cpu(evsel, evsel__cpus(evsel));
> + return perf_evsel__open_per_cpu(evsel, evsel__cpus(evsel), cpu);
>
> return perf_evsel__open_per_thread(evsel, evsel->core.threads);
> }
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 081c4a5113c6..4c9a7b68c3e7 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -213,7 +213,8 @@ size_t perf_event__fprintf_stat_config(union perf_event *event, FILE *fp);
>
> int create_perf_stat_counter(struct evsel *evsel,
> struct perf_stat_config *config,
> - struct target *target);
> + struct target *target,
> + int cpu);
> void
> perf_evlist__print_counters(struct evlist *evlist,
> struct perf_stat_config *config,
> --
> 2.21.0
>
Powered by blists - more mailing lists