Re: [PATCH v2 06/12] perf record: Switch user option to use BPF filter

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Apr 10, 2025 at 10:36:25AM -0700, Ian Rogers wrote:
> Finding user processes by scanning /proc is inherently racy and
> results in perf_event_open failures. Use a BPF filter to drop samples
> where the uid doesn't match. Ensure adding the BPF filter forces
> system-wide.

Since the BPF filter is not introduced in this patch, can you please
provide, in the commit log message or in the patch itself, some
commentary as to how this is accomplished thru a BPF filter?

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-record.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index ba20bf7c011d..202c917fd122 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -173,6 +173,7 @@ struct record {
>  	bool			timestamp_boundary;
>  	bool			off_cpu;
>  	const char		*filter_action;
> +	const char		*uid_str;
>  	struct switch_output	switch_output;
>  	unsigned long long	samples;
>  	unsigned long		output_max_size;	/* = 0: unlimited */
> @@ -3460,8 +3461,7 @@ static struct option __record_options[] = {
>  		     "or ranges of time to enable events e.g. '-D 10-20,30-40'",
>  		     record__parse_event_enable_time),
>  	OPT_BOOLEAN(0, "kcore", &record.opts.kcore, "copy /proc/kcore"),
> -	OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
> -		   "user to profile"),
> +	OPT_STRING('u', "uid", &record.uid_str, "user", "user to profile"),
>  
>  	OPT_CALLBACK_NOOPT('b', "branch-any", &record.opts.branch_stack,
>  		     "branch any", "sample any taken branches",
> @@ -4196,19 +4196,24 @@ int cmd_record(int argc, const char **argv)
>  		ui__warning("%s\n", errbuf);
>  	}
>  
> -	err = target__parse_uid(&rec->opts.target);
> -	if (err) {
> -		int saved_errno = errno;
> +	if (rec->uid_str) {
> +		uid_t uid = parse_uid(rec->uid_str);
>  
> -		target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
> -		ui__error("%s", errbuf);
> +		if (uid == UINT_MAX) {
> +			ui__error("Invalid User: %s", rec->uid_str);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +		err = parse_uid_filter(rec->evlist, uid);
> +		if (err)
> +			goto out;
>  
> -		err = -saved_errno;
> -		goto out;
> +		/* User ID filtering implies system wide. */
> +		rec->opts.target.system_wide = true;
>  	}
>  
> -	/* Enable ignoring missing threads when -u/-p option is defined. */
> -	rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX || rec->opts.target.pid;
> +	/* Enable ignoring missing threads when -p option is defined. */
> +	rec->opts.ignore_missing_thread = rec->opts.target.pid;
>  
>  	evlist__warn_user_requested_cpus(rec->evlist, rec->opts.target.cpu_list);
>  
> -- 
> 2.49.0.604.gff1f9ca942-goog