Re: [RESEND PATCH v5 06/12] smp: Enable preemption early in smp_call_function_many_cond

["Chuyi Zhou" <zhouchuyi@bytedance.com>]

On 2026-05-22 6:08 p.m., Sebastian Andrzej Siewior wrote:
> On 2026-05-13 20:45:18 [+0800], Chuyi Zhou wrote:
>> --- a/kernel/smp.c
>> +++ b/kernel/smp.c
>> @@ -861,10 +861,10 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
>>   	int nr_cpus = 0;
>>   	bool run_remote = false;
>>   
>> -	lockdep_assert_preemption_disabled();
>> -
>>   	task_mask = smp_task_ipi_mask(current);
>> -	preemptible_wait = task_mask;
>> +	preemptible_wait = task_mask && preemptible();
> 
> Now that I stare at this again, why is preemptible() a thing here?
> You care about doing put_cpu() below before csd_lock_wait(). This can
> only be done if you get a cpumask from smp_task_ipi_mask(). If
> preemption or interrupts is/are disabled then you still can use the
> "private" cpumask and do the early put_cpu(). It simply is no
> optimisation.
> 
> This basically reduces the check to a CONFIG_PREEMPTION=y kernel because
> otherwise you have no cpumask. And this is not done on !SMP kernels.
> smp_task_ipi_mask() is only used here so there is no need to export it
> via headers.
> 


Agreed. The safety condition for the early put_cpu() is the availability
of the task-local cpumask, not preemptible(). If an outer context 
already keeps preemption or interrupts disabled, the early put_cpu() 
simply does not buy anything.

I will make the condition depend on task_mask directly, drop the
preemptible() check, keep smp_task_ipi_mask() local to kernel/smp.c, and
adjust the comment as suggested.

   Thanks.


>> +
>> +	this_cpu = get_cpu();
>>   	cfd = this_cpu_ptr(&cfd_data);
>>   	cpumask = preemptible_wait ? task_mask : cfd->cpumask;
>>   
>> @@ -946,6 +946,19 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
>>   		local_irq_restore(flags);
>>   	}
>>   
>> +	/*
>> +	 * We may block in csd_lock_wait() for a significant amount of time,
>> +	 * especially when interrupts are disabled or with a large number of
>> +	 * remote CPUs. Try to enable preemption before csd_lock_wait().
> 
> If interrupts are disabled there is no gain. Also we sort of expect
> interrupts to be enabled here.
> 
>     Waiting for completion can take time especially with many CPUs. On a
>     PREEMPTIBLE kernel a per-task cpumask is used to track CPUs with
>     pending IPI request. This allows to enable preemption and potentially
>     wait while allowing task preemption. On a !PREEMPTIBLE the cpumask is
>     shared and the call must block until completion to avoid modifications
>     by a another caller on this CPU.
> 
>> +	 *
>> +	 * Use the task_mask instead of cfd->cpumask to avoid concurrency
>> +	 * modification from tasks on the same cpu. If preemption occurs during
>> +	 * csd_lock_wait, other concurrent smp_call_function_many_cond() calls
>> +	 * will simply block until the previous csd->func() completes.
>> +	 */
>> +	if (preemptible_wait)
>> +		put_cpu();
>> +
>>   	if (run_remote && wait) {
>>   		for_each_cpu(cpu, cpumask) {
>>   			call_single_data_t *csd;
>> @@ -954,6 +967,9 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
>>   			csd_lock_wait(csd);
>>   		}
>>   	}
>> +
>> +	if (!preemptible_wait)
>> +		put_cpu();
>>   }
>>   
>>   /**
> 
> Sebastian