Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 30 Mar 2022 15:50:36 +0200
From:   Jiri Olsa <olsajiri@gmail.com>
To:     Shunsuke Nakamura <nakamura.shun@fujitsu.com>
Cc:     peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
        mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
        jolsa@redhat.com, namhyung@kernel.org,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [RFC PATCH v2 5/7] libperf: Add perf_evsel__check_overflow()
 functions
Message-ID: <YkRgLBTHVQbdX8rK@krava>
References: <20220325043829.224045-1-nakamura.shun@fujitsu.com>
 <20220325043829.224045-6-nakamura.shun@fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220325043829.224045-6-nakamura.shun@fujitsu.com>
Precedence: bulk

On Fri, Mar 25, 2022 at 01:38:27PM +0900, Shunsuke Nakamura wrote:
> Add the following functions:
> 
>   perf_evsel__check_overflow
>   perf_evsel__check_overflow_cpu
> 
> to check for perf events with the file descriptor specified in the
> argument.
> These functions can be used in signal handlers to check overflow.
> 
> Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
> ---
>  tools/lib/perf/Documentation/libperf.txt |  3 +++
>  tools/lib/perf/evsel.c                   | 34 ++++++++++++++++++++++++
>  tools/lib/perf/include/perf/evsel.h      |  4 +++
>  tools/lib/perf/libperf.map               |  2 ++
>  4 files changed, 43 insertions(+)
> 
> diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> index 700c1ce15159..4ae8d738b948 100644
> --- a/tools/lib/perf/Documentation/libperf.txt
> +++ b/tools/lib/perf/Documentation/libperf.txt
> @@ -165,6 +165,9 @@ SYNOPSIS
>                                int cpu_map_idx);
>    int perf_evsel__period(struct perf_evsel *evsel, int period);
>    int perf_evsel__period_cpu(struct perf_evsel *evsel, int period, int cpu_map_idx);
> +  int perf_evsel__check_overflow(struct perf_evsel *evsel, int sig_fd, bool *overflow);
> +  int perf_evsel__check_overflow_cpu(struct perf_evsel *evsel, int cpu_map_idx,
> +                                     int sig_fd, bool *overflow);

should this be more like:

  perf_evsel__has_fd(struct perf_evsel *evsel, int fd)

also why do we need to export *_cpu version for this?
I don't see it used in the test

>    struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
>    struct perf_thread_map *perf_evsel__threads(struct perf_evsel *evsel);
>    struct perf_event_attr *perf_evsel__attr(struct perf_evsel *evsel);
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index db9b7274feb5..6ff3cf692df3 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -616,3 +616,37 @@ int perf_evsel__open_opts(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
>  
>  	return err;
>  }
> +
> +int perf_evsel__check_overflow_cpu(struct perf_evsel *evsel, int cpu_map_idx,
> +				   int sig_fd, bool *overflow)
> +{
> +	int thread;
> +	int *fd;
> +	int err = 0;
> +
> +	if (!overflow)
> +		return -EINVAL;
> +
> +	*overflow = false;
> +
> +	for (thread = 0; thread < xyarray__max_y(evsel->fd) && !err; ++thread) {
> +		fd = FD(evsel, cpu_map_idx, thread);
> +		if (sig_fd <= 0 || !fd || *fd < 0)
> +			err = -EINVAL;

sig_fd check should be before the loop

jirka