[PATCH v3 05/12] smp: Free call_function_data via RCU in smpcfd_dead_cpu

Chuyi Zhou posted 12 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH v3 05/12] smp: Free call_function_data via RCU in smpcfd_dead_cpu
Posted by Chuyi Zhou 2 weeks, 5 days ago
Use rcu_read_lock() to protect csd in smp_call_function_many_cond() and
wait for all read critical sections to exit before releasing percpu csd
data. This is preparation for enabling preemption during csd_lock_wait()
and can prevent accessing cfd->csd data that has already been freed in
smpcfd_dead_cpu().

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 kernel/smp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/smp.c b/kernel/smp.c
index 9728ba55944d..32c293d8be0e 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -77,6 +77,7 @@ int smpcfd_dead_cpu(unsigned int cpu)
 {
 	struct call_function_data *cfd = &per_cpu(cfd_data, cpu);
 
+	synchronize_rcu();
 	free_cpumask_var(cfd->cpumask);
 	free_cpumask_var(cfd->cpumask_ipi);
 	free_percpu(cfd->csd);
@@ -810,6 +811,7 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
 
 	lockdep_assert_preemption_disabled();
 
+	rcu_read_lock();
 	cfd = this_cpu_ptr(&cfd_data);
 	cpumask = cfd->cpumask;
 
@@ -905,6 +907,7 @@ static void smp_call_function_many_cond(const struct cpumask *mask,
 		}
 	}
 
+	rcu_read_unlock();
 	if (preemptible_wait)
 		free_cpumask_var(cpumask_stack);
 }
-- 
2.20.1
Re: [PATCH v3 05/12] smp: Free call_function_data via RCU in smpcfd_dead_cpu
Posted by Sebastian Andrzej Siewior 2 weeks, 5 days ago
On 2026-03-18 12:56:31 [+0800], Chuyi Zhou wrote:
> Use rcu_read_lock() to protect csd in smp_call_function_many_cond() and
> wait for all read critical sections to exit before releasing percpu csd
> data. This is preparation for enabling preemption during csd_lock_wait()
> and can prevent accessing cfd->csd data that has already been freed in
> smpcfd_dead_cpu().
> 
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
> ---
>  kernel/smp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 9728ba55944d..32c293d8be0e 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -77,6 +77,7 @@ int smpcfd_dead_cpu(unsigned int cpu)
>  {
>  	struct call_function_data *cfd = &per_cpu(cfd_data, cpu);
>  
> +	synchronize_rcu();
>  	free_cpumask_var(cfd->cpumask);
>  	free_cpumask_var(cfd->cpumask_ipi);
>  	free_percpu(cfd->csd);

This seems to make sense. But it could delay CPU shutdown and then the
stress-cpu-hotplug.sh. And this one helped to find bugs.

What is expectation of shutting down a CPU? Will it remain off for
_long_ at which point we care about this memory or is temporary and we
could skip to free the memory here because we allocate it only once on
the UP side?

Sebastian
Re: [PATCH v3 05/12] smp: Free call_function_data via RCU in smpcfd_dead_cpu
Posted by Chuyi Zhou 2 weeks, 4 days ago
On 2026-03-19 12:19 a.m., Sebastian Andrzej Siewior wrote:
> On 2026-03-18 12:56:31 [+0800], Chuyi Zhou wrote:
>> Use rcu_read_lock() to protect csd in smp_call_function_many_cond() and
>> wait for all read critical sections to exit before releasing percpu csd
>> data. This is preparation for enabling preemption during csd_lock_wait()
>> and can prevent accessing cfd->csd data that has already been freed in
>> smpcfd_dead_cpu().
>>
>> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
>> ---
>>   kernel/smp.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/smp.c b/kernel/smp.c
>> index 9728ba55944d..32c293d8be0e 100644
>> --- a/kernel/smp.c
>> +++ b/kernel/smp.c
>> @@ -77,6 +77,7 @@ int smpcfd_dead_cpu(unsigned int cpu)
>>   {
>>   	struct call_function_data *cfd = &per_cpu(cfd_data, cpu);
>>   
>> +	synchronize_rcu();
>>   	free_cpumask_var(cfd->cpumask);
>>   	free_cpumask_var(cfd->cpumask_ipi);
>>   	free_percpu(cfd->csd);
> 
> This seems to make sense. But it could delay CPU shutdown and then the
> stress-cpu-hotplug.sh. And this one helped to find bugs.
> 
> What is expectation of shutting down a CPU? Will it remain off for
> _long_ at which point we care about this memory or is temporary and we
> could skip to free the memory here because we allocate it only once on
> the UP side?
> 


Yes, we can allocate the csd only once during the first UP side. The 
advantage is that it avoids delaying the CPU offline process and keeps 
the code simpler.

The tradeoff is a slight memory waste in scenarios where the CPU remains 
offline for a long time(or never online again). If we cannot tolerate 
the delay caused by synchronize_rcu() during the CPU offline process, we 
can switch to approach you suggested.

Thanks.

> Sebastian