Re: [PATCH 2/6] kthread: Keep unbound kthreads off nohz_full CPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kthread: offline preferred CPUs bypass hk_kthread in kthread_fetch_affinity()
--

commit e454a827bc8a9ce4259c7edb902592efbf81ab53
Author: Caron Wills <caron.wills@pulsar.com>

kthread: Keep unbound kthreads off nohz_full CPUs

This commit intersects unbound kthread affinity with both HK_TYPE_DOMAIN and
HK_TYPE_KERNEL_NOISE in kthread_fetch_affinity() so that unbound kthreads are
kept off nohz_full CPUs even when those CPUs remain in scheduler domains.

Link: https://bugs.launchpad.net/bugs/2165410

> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index fdb8fb07f40bd..b9d8dfe1b6cad 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -327,24 +327,46 @@ void __noreturn kthread_complete_and_exit(struct completion *comp, long code)
>  }
>  EXPORT_SYMBOL(kthread_complete_and_exit);
>  
> +/*
> + * Unbound kthreads must run neither on domain isolated CPUs (isolcpus=domain,
> + * cpuset isolated partitions) nor on CPUs isolated from kernel noise
> + * (nohz_full=, isolcpus=nohz). The two sets usually coincide, but the kernel
> + * accepts nohz_full CPUs that remain in the scheduler domains, so both
> + * housekeeping types have to be honoured.
> + */
>  static void kthread_fetch_affinity(struct kthread *kthread, struct cpumask *cpumask)
>  {
> -	const struct cpumask *pref;
> +	const struct cpumask *pref, *hk_domain, *hk_kthread;
>  
>  	guard(rcu)();
>  
> +	hk_domain = housekeeping_cpumask(HK_TYPE_DOMAIN);
> +	/*
> +	 * Deliberately not HK_TYPE_KTHREAD: since v7.1 that is an alias of
> +	 * HK_TYPE_DOMAIN (commit 8f78b749f3da), which is the very assumption
> +	 * being corrected here.
> +	 */
> +	hk_kthread = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE);
> +
>  	if (kthread->preferred_affinity) {
>  		pref = kthread->preferred_affinity;
>  	} else {
>  		if (kthread->node == NUMA_NO_NODE)
> -			pref = housekeeping_cpumask(HK_TYPE_DOMAIN);
> +			pref = hk_domain;
>  		else
>  			pref = cpumask_of_node(kthread->node);
>  	}
>  
> -	cpumask_and(cpumask, pref, housekeeping_cpumask(HK_TYPE_DOMAIN));
> +	cpumask_and(cpumask, pref, hk_domain);
> +	cpumask_and(cpumask, cpumask, hk_kthread);
> +	if (cpumask_empty(cpumask))
> +		cpumask_and(cpumask, hk_domain, hk_kthread);
> +	/*
> +	 * Both the boot parameters and cpuset guarantee that at least one
> +	 * CPU stays housekeeping for every type, but don't rely on it.
> +	 */
>  	if (!cpumask_intersects(cpumask, cpu_online_mask))
> -		cpumask_copy(cpumask, housekeeping_cpumask(HK_TYPE_DOMAIN));
> +		cpumask_copy(cpumask, hk_domain);

[Severity: High]
What happens in kthread_fetch_affinity() when the intersection of pref,
hk_domain, and hk_kthread is non-empty, but contains only offline CPUs?

Commit 78ff77ded78f ("kthread: Fall back when no preferred housekeeping CPU
is online") replaced cpumask_empty(cpumask) with
!cpumask_intersects(cpumask, cpu_online_mask) because preferred_affinity can
contain possible CPUs that are offline. For example, rcu_thread_affine_rnp()
populates preferred_affinity with all possible CPUs of the leaf rcu_node:

kernel/rcu/tree.c:rcu_thread_affine_rnp() {
	...
	for_each_leaf_node_possible_cpu(rnp, cpu)
		cpumask_set_cpu(cpu, affinity);

	kthread_affine_preferred(t, affinity);
	...
}

If a leaf rcu_node has online CPUs that are all in nohz_full, plus offline
possible CPUs that were not listed in nohz_full=, or if the preferred
housekeeping CPUs are later hot-unplugged, then pref & hk_domain & hk_kthread
will contain only those offline CPUs.

In that case:
1. cpumask_empty(cpumask) is false, so the fallback to
   cpumask_and(cpumask, hk_domain, hk_kthread) is skipped.
2. !cpumask_intersects(cpumask, cpu_online_mask) is true, so cpumask is
   overwritten with hk_domain.

On systems where nohz_full CPUs remain in HK_TYPE_DOMAIN, wouldn't this
bypass hk_kthread and allow the kthread to be affined to nohz_full CPUs even
when online CPUs in hk_domain & hk_kthread exist elsewhere on the system?

Should the first fallback check !cpumask_intersects(cpumask, cpu_online_mask)
instead of cpumask_empty(cpumask)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103075940.110308-1-matthew.ruffell@canonical.com?part=2