Re: [PATCH 27/28] perf kwork: Bounds check work->cpu before indexing cpus_runtime[]

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch double-counts skipped work entries, artificially inflating the reported warning metric, and introduces dead code.
--

commit 1c5f60cee04ed4d88c1c3eeee79849a9dd16ec72
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf kwork: Bounds check work->cpu before indexing cpus_runtime[]

This commit replaces a signed BUG_ON with an unsigned bounds check to
safely skip processing work entries with invalid CPU values. It guards
accumulators and usage calculations to prevent out-of-bounds accesses on
cpus_runtime[].

> diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> index 9d3a4c779a41e..6e124a0f277c8 100644
> --- a/tools/perf/builtin-kwork.c
> +++ b/tools/perf/builtin-kwork.c

[ ... ]

> @@ -2032,6 +2046,12 @@ static void top_calc_irq_runtime(struct perf_kwork *kwork,
>  				 enum kwork_class_type type,
>  				 struct kwork_work *work)
>  {
>  	struct kwork_top_stat *stat = &kwork->top_stat;
>  
> +	/* See comment in top_calc_total_runtime() */
> +	if ((unsigned int)work->cpu >= MAX_NR_CPUS) {
> +		stat->nr_skipped_cpu++;
> +		return;
> +	}

Is this bounds check unreachable? top_calc_irq_runtime() is invoked 
exclusively from top_subtract_irq_runtime(), which retrieves the IRQ 
task based on the work->cpu provided by top_calc_cpu_usage(). 

Since top_calc_cpu_usage() already skips invalid CPUs before calling the 
subtraction logic, it seems this code might never be executed.

[ ... ]

> @@ -2084,12 +2104,21 @@ static void top_calc_cpu_usage(struct perf_kwork *kwork)
>  		if (work->total_runtime == 0)
>  			goto next;
>  
> +		/* See comment in top_calc_total_runtime() */
> +		if ((unsigned int)work->cpu >= MAX_NR_CPUS) {
> +			stat->nr_skipped_cpu++;
> +			goto next;
> +		}

Does this result in double-counting the skipped entries? Both 
top_calc_total_runtime() and top_calc_cpu_usage() iterate over 
class->work_root independently.

Because top_calc_total_runtime() also increments stat->nr_skipped_cpu, 
doing it again here will cause the warning metric reported to the user 
to be artificially doubled for any invalid nodes with a total_runtime 
greater than 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510033424.255812-1-acme@kernel.org?part=27