| 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: <CAM9d7cjqP=M=V2neRC8WCQ-xAJKK0H21LK6vMo3MPed-QQ5WcA@mail.gmail.com>
Date: Fri, 20 May 2022 12:28:29 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Shunsuke Nakamura <nakamura.shun@...itsu.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@...hat.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-perf-users <linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH 3/7] libperf: Add support for overflow handling of
sampling events
On Fri, Apr 22, 2022 at 2:44 AM Shunsuke Nakamura
<nakamura.shun@...itsu.com> wrote:
>
> Extend the fields of the opts structure to set up overflow handling
> for sampling events. Also, add processing to set signal handlers in
> perf_evsel__open_opts.
>
> Signed-off-by: Shunsuke Nakamura <nakamura.shun@...itsu.com>
> ---
> tools/lib/perf/Documentation/libperf.txt | 6 +-
> tools/lib/perf/Makefile | 1 +
> tools/lib/perf/evsel.c | 79 ++++++++++++++++++++++++
> tools/lib/perf/include/perf/evsel.h | 6 +-
> tools/lib/perf/tests/test-evlist.c | 1 -
> 5 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> index 6ca91ca94e01..ec93b1c75ebe 100644
> --- a/tools/lib/perf/Documentation/libperf.txt
> +++ b/tools/lib/perf/Documentation/libperf.txt
> @@ -137,8 +137,12 @@ SYNOPSIS
> size_t sz;
>
> unsigned long open_flags; /* perf_event_open flags */
> + int flags; /* fcntl flags */
What about just rename it to fcntl_flags ?
> + unsigned int signal;
int ?
> + int owner_type;
Please add a comment that it's for F_SETOWN_EX.
> + struct sigaction *sig;
> };
> - #define perf_evsel_open_opts__last_field open_flags
> + #define perf_evsel_open_opts__last_field sig
>
> #define LIBPERF_OPTS(TYPE, NAME, ...)
>
> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
> index 21df023a2103..4c8fae193cf5 100644
> --- a/tools/lib/perf/Makefile
> +++ b/tools/lib/perf/Makefile
> @@ -75,6 +75,7 @@ override CFLAGS += -Werror -Wall
> override CFLAGS += -fPIC
> override CFLAGS += $(INCLUDES)
> override CFLAGS += -fvisibility=hidden
> +override CFLAGS += -D_GNU_SOURCE
>
> all:
>
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 00c0cea43b52..a289f6c44d7c 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -460,6 +460,79 @@ void perf_counts_values__scale(struct perf_counts_values *count,
> *pscaled = scaled;
> }
>
> +static int perf_evsel__run_fcntl(struct perf_evsel *evsel,
> + unsigned int cmd, unsigned long arg,
> + int cpu_map_idx)
I think it'd be better to have _cpu suffix in the function name
as it handles on the specific cpu.
> +{
> + int thread;
> +
> + for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
> + int err;
> + int *fd = FD(evsel, cpu_map_idx, thread);
> +
> + if (!fd || *fd < 0)
> + return -1;
> +
> + err = fcntl(*fd, cmd, arg);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int perf_evsel__set_signal_handler(struct perf_evsel *evsel,
> + struct perf_evsel_open_opts *opts)
> +{
> + unsigned int flags;
> + unsigned int signal;
> + struct f_owner_ex owner;
> + struct sigaction *sig;
It can be confused with the signal number, how about sigact?
> + int cpu_map_idx;
> + int err = 0;
> +
> + flags = OPTS_GET(opts, flags, (O_RDWR | O_NONBLOCK | O_ASYNC));
> + signal = OPTS_GET(opts, signal, SIGIO);
> + owner.type = OPTS_GET(opts, owner_type, F_OWNER_PID);
> + sig = OPTS_GET(opts, sig, NULL);
> +
> + if (flags == 0 && signal == 0 && !owner.type == 0 && sig == 0)
You man want to check owner.type without "!".
> + return err;
> +
> + err = sigaction(signal, sig, NULL);
> + if (err)
> + return err;
> +
> + switch (owner.type) {
> + case F_OWNER_PID:
> + owner.pid = getpid();
> + break;
> + case F_OWNER_TID:
> + owner.pid = syscall(SYS_gettid);
> + break;
> + case F_OWNER_PGRP:
> + default:
> + return -1;
> + }
> +
> + for (cpu_map_idx = 0; cpu_map_idx < xyarray__max_x(evsel->fd); cpu_map_idx++) {
> + err = perf_evsel__run_fcntl(evsel, F_SETFL, flags, cpu_map_idx);
> + if (err)
> + return err;
> +
> + err = perf_evsel__run_fcntl(evsel, F_SETSIG, signal, cpu_map_idx);
> + if (err)
> + return err;
> +
> + err = perf_evsel__run_fcntl(evsel, F_SETOWN_EX,
> + (unsigned long)&owner, cpu_map_idx);
> + if (err)
> + return err;
> + }
> +
> + return err;
> +}
> +
> int perf_evsel__open_opts(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
> struct perf_thread_map *threads,
> struct perf_evsel_open_opts *opts)
> @@ -474,6 +547,12 @@ int perf_evsel__open_opts(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
> evsel->open_flags = OPTS_GET(opts, open_flags, 0);
>
> err = perf_evsel__open(evsel, cpus, threads);
> + if (err)
> + return err;
> +
> + err = perf_evsel__set_signal_handler(evsel, opts);
> + if (err)
> + return err;
>
> return err;
> }
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index 1140df4d2578..50662babfe97 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -29,8 +29,12 @@ struct perf_evsel_open_opts {
> size_t sz;
>
> unsigned long open_flags; /* perf_event_open flags */
> + int flags; /* fcntl flags */
> + unsigned int signal;
> + int owner_type;
> + struct sigaction *sig;
> };
> -#define perf_evsel_open_opts__last_field open_flags
> +#define perf_evsel_open_opts__last_field sig
>
> #define LIBPERF_OPTS(TYPE, NAME, ...) \
> struct TYPE NAME = ({ \
> diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
> index ed616fc19b4f..d8f9493cd4d1 100644
> --- a/tools/lib/perf/tests/test-evlist.c
> +++ b/tools/lib/perf/tests/test-evlist.c
> @@ -1,5 +1,4 @@
> // SPDX-License-Identifier: GPL-2.0
> -#define _GNU_SOURCE // needed for sched.h to get sched_[gs]etaffinity and CPU_(ZERO,SET)
Could be in a separate change.
Thanks,
Namhyung
> #include <inttypes.h>
> #include <sched.h>
> #include <stdio.h>
> --
> 2.25.1
>
Powered by blists - more mailing lists