Re: [PATCH bpf v2 2/2] bpftool: Fix sparse CPU IDs in prog profile

[bot+bpf-ci]

>     Tested:
>     - Built tools/bpf/bpftool successfully on the host.
>     - Booted an arm64 QEMU guest with a patched virt device tree reporting
>       possible=0,2-3, present=0,2-3, and online=0,2-3.
>     - Ran both pre-fix and fixed bpftool with cycles and instructions against
>       a BTF-enabled fentry target. The pre-fix binary faulted in
>       perf_event_alloc(), while the fixed binary reached perf-event setup and
>       reported failure to create the instructions event on CPU 0. QEMU did
>       not provide a usable hardware PMU runtime result, so no profile counts
>       are claimed.

This isn't a bug, but would it read better to move the Tested: block
below the --- line, or fold it into a shorter sentence, so the changelog
itself stays focused on the sparse-mask problem and the new stride? The
block is the longest section of the message while reporting that the run
did not produce a usable result ("reported failure to create the
instructions event on CPU 0", "QEMU did not provide a usable hardware
PMU runtime result, so no profile counts are claimed").

> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index a9f730d407a92..bb969ffce453d 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c

[ ... ]

> @@ -2440,7 +2449,8 @@ static void int_exit(int signo)
>
>  static int do_profile(int argc, char **argv)
>  {
> -	int num_metric, num_cpu, err = -1;
> +	int num_metric, err = -1;
> +	int *cpu_ids = NULL;
>  	struct bpf_program *prog;
>  	unsigned long duration;
>  	char *endptr;

[ ... ]

> @@ -2471,11 +2481,13 @@ static int do_profile(int argc, char **argv)
>  	if (num_metric <= 0)
>  		goto out;
>
> -	num_cpu = libbpf_num_possible_cpus();
> -	if (num_cpu <= 0) {
> +	profile_cpu_cnt = get_possible_cpu_ids(&cpu_ids);
> +	if (profile_cpu_cnt <= 0) {
>  		p_err("failed to identify number of CPUs");
>  		goto out;
>  	}
> +	profile_cpu_ids = cpu_ids;
> +	profile_cpu_id_span = cpu_ids[profile_cpu_cnt - 1] + 1;

This isn't a bug, but could this pass &profile_cpu_ids straight into
get_possible_cpu_ids() and drop the local plus its initializer, since
the helper already clears the output pointer on entry? The local cpu_ids
exists only to be copied into the profile_cpu_ids static on the next
line, and its = NULL initializer is never observed because
get_possible_cpu_ids() unconditionally does *cpu_ids = NULL as its first
statement.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35336124623