Re: [PATCH] lib/smp_processor_id: Don't warn if there is only one CPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] lib/smp_processor_id: check_preemption_disabled() disables
  checks when 1 CPU
- [Low] lib/smp_processor_id: missing Fixes trailer on stable bug-fix
  commit
--

commit 0181ac5af2fbbf50ee4ee229425b805d588cbf1c
Author: Huacai Chen <chenhuacai@kernel.org>

> lib/smp_processor_id: Don't warn if there is only one CPU
>
> When CONFIG_DEBUG_PREEMPT is enabled on a Non-SMP (single-core) kernel,
> calls to smp_processor_id() in preemptible contexts trigger some false
> positive "BUG: using smp_processor_id() in preemptible" warnings, which
> are harmless noise because the process always executes on CPU 0 and CPU
> migration is impossible.
>
> Here are some of the trimmed log messages:
>
>   Call Trace:
>   [<9000000000d33e28>] show_stack+0xf8/0x1c0
>   [<9000000000d2fdac>] dump_stack_lvl+0x4c/0x78
>   [<9000000002456fc0>] check_preemption_disabled.isra.0+0x100/0x110
>   [<9000000002456ff8>] debug_smp_processor_id+0x18/0x40
>   [<9000000000d31118>] init_cache_level+0x18/0x90
>   [<9000000001adbe2c>] detect_cache_attributes+0x4c/0x640
>   [<9000000001adc44c>] cacheinfo_cpu_online+0x1c/0x310
>   [<9000000000d5ae04>] cpuhp_issue_call.constprop.0+0x94/0xd0
>   [<9000000000d5b278>] __cpuhp_setup_state_cpuslocked+0x208/0x320
>   [<9000000000d30148>] do_one_initcall+0x68/0x200
>   [<90000000024711b8>] kernel_init_freeable+0x1fc/0x294
>   [<900000000245824c>] kernel_init+0x20/0x134
>   [<9000000002455b54>] ret_from_kernel_thread+0x24/0x1d0
>   [<9000000000d31650>] ret_from_kernel_thread_asm+0xc/0x9c
>
> This warning only exists for the !CONFIG_SMP build. This is because for
> CONFIG_SMP cpuhp_issue_call() invokes callbacks in cpuhp_thread_fun()
> which is a percpu thread, but for !CONFIG_SMP build it invoke callbacks
> in the init process which is not a percpu thread. And there is such a
> skip condition in check_preemption_disabled():
>
>     if (is_percpu_thread())
>             goto out;
>
> Since there is only one CPU for the !CONFIG_SMP, we can simply skip the
> warning if (nr_cpu_ids == 1) in check_preemption_disabled().
>
> Cc: stable@vger.kernel.org
> Reported-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>

[Severity: Low]
This commit is tagged for stable backporting to fix the !CONFIG_SMP warning
exposed when is_percpu_thread() was made unconditional of CONFIG_SMP.
Should it include a Fixes: tag such as:

  Fixes: 06ddd17521bf ("sched/smp: Always define is_percpu_thread() and scheduler_ipi()")

> diff --git a/lib/smp_processor_id.c b/lib/smp_processor_id.c
> index 94b3f6b195388..64901d33c0eee 100644
> --- a/lib/smp_processor_id.c
> +++ b/lib/smp_processor_id.c
> @@ -19,6 +19,9 @@ unsigned int check_preemption_disabled(const char *what1, const char *what2)
>  	if (irqs_disabled())
>  		goto out;
>  
> +	if (nr_cpu_ids == 1)
> +		goto out;
> +
>  	if (is_percpu_thread())
>  		goto out;

[Severity: Medium]
Does returning early when nr_cpu_ids == 1 disable CONFIG_DEBUG_PREEMPT
checks across the entire kernel on single-CPU systems?

check_preemption_disabled() also backs __this_cpu_preempt_check().  Even
with a single CPU, preemptible code using __this_cpu_*() or per-CPU state
via smp_processor_id() can be preempted on CPU 0 by another task that
modifies the same per-CPU variable.

In addition, nr_cpu_ids == 1 is true at runtime on CONFIG_SMP=y kernels
booted on a 1-vCPU VM or with nr_cpus=1, and on !CONFIG_SMP builds
nr_cpu_ids is a compile-time constant 1, making the rest of
check_preemption_disabled() dead code and undoing 2885daf47081
("lib/smp_processor_id: Make migration check unconditional of SMP").

The reported warning happens because cpuhp_issue_call() on !CONFIG_SMP
invokes non-atomic hotplug callbacks directly in the calling task without
migrate_disable(), and init_cache_level() in
arch/loongarch/kernel/cacheinfo.c reads current_cpu_data instead of
cpu_data[cpu].  Could this be fixed in cpuhp_issue_call() or
arch/loongarch/kernel/cacheinfo.c instead of disabling
check_preemption_disabled()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111092421.3771992-1-chenhuacai@loongson.cn?part=1