Re: [RESEND PATCH v5 04/12] smp: Use task-local IPI cpumask in smp_call_function_many_cond()

[Sebastian Andrzej Siewior]

On 2026-05-22 00:00:25 [+0800], Chuyi Zhou wrote:
> On 2026-05-21 11:26 p.m., Sebastian Andrzej Siewior wrote:
> > On 2026-05-13 20:45:16 [+0800], Chuyi Zhou wrote:
> >> diff --git a/kernel/smp.c b/kernel/smp.c
> >> index 9e9dab3b0d51..630c8e5a635d 100644
> >> --- a/kernel/smp.c
> >> +++ b/kernel/smp.c
> >> @@ -811,11 +849,18 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
> >>   	int cpu, last_cpu, this_cpu = smp_processor_id();
> >>   	struct call_function_data *cfd;
> >>   	bool wait = scf_flags & SCF_WAIT;
> >> +	struct cpumask *cpumask, *task_mask;
> >> +	bool preemptible_wait;
> >>   	int nr_cpus = 0;
> >>   	bool run_remote = false;
> >>   
> >>   	lockdep_assert_preemption_disabled();
> >>   
> >> +	task_mask = smp_task_ipi_mask(current);
> >> +	preemptible_wait = task_mask;
> >> +	cfd = this_cpu_ptr(&cfd_data);
> >> +	cpumask = preemptible_wait ? task_mask : cfd->cpumask;
> > 
> > Is this a porting artifact? preemptible_wait is a bool while task_mask
> > is a pointer which is always != NULL.
> > 
> 
> Not exactly. The NULL fallback is intentional: smp_task_ipi_mask()
> returns NULL for !CONFIG_PREEMPTION, and init_task may also not have an
> allocated ipi_mask_ptr in the non-inlined case.

Okay, but please don't assign a cpumask to a bool and having one to
guess all that behind it. Something like

	cfd = this_cpu_ptr(&cfd_data);
	task_mask = smp_task_ipi_mask(current);
	if (task_mask)
		cpumask = task_mask;
	else
         	cpumask = cfd->cpumask;

would make it probably obvious. You probably need a bool later on to
check if you use task's mask and can open preemption while waiting. Hmm.
Let me continue…

> But you are right that the name is confusing in this preparatory patch.
> Here it only means "task-local cpumask is available", not that the wait is
> actually preemptible. I will drop this bool in this patch, and keep
> the real preemptible wait condition in the later patch as:
> 
>     preemptible_wait = task_mask && preemptible();

There is a lockdep_assert_preemption_disabled() so that preemptible()
must evaluate false.

Sebastian