Re: [PATCH v9 04/14] smp: Use task-local IPI cpumask in smp_call_function_many_cond()

[Chuyi Zhou]

On 2026-07-08 7:17 a.m., Thomas Gleixner wrote:
> On Tue, Jun 30 2026 at 19:09, Chuyi Zhou wrote:
>> The memory cost is explicit: one word is added to task_struct. When
>> cpumask_size() fits in that word, the mask is stored inline and no
>> separate allocation is needed. Larger systems allocate cpumask_size() per
>> task; on x86-64 NR_CPUS=8192 this is about 1 KiB per task. For context,
> 
> It's not about. It's one kilobyte, no?
> 
>> @@ -1364,6 +1364,12 @@ struct task_struct {
>>   	struct list_head		perf_event_list;
>>   	struct perf_ctx_data __rcu	*perf_ctx_data;
>>   #endif
>> +#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPTION)
>> +	union {
>> +		cpumask_t			*ipi_mask_ptr;
>> +		unsigned long			ipi_mask_val;
>> +	};
>> +#endif
> 
> Aside of this horrible ifdeffery, this construct makes me more than
> nervous as it's too trivial to add code which deferences the pointer
> directly. The proper thing to do is:
> 
> #if defined(CONFIG_SMP) && defined(CONFIG_PREEMPTION)
> struct task_ipi_mask {
> 	union {
> 		cpumask_t			*ipi_mask_ptr;
> 		unsigned long			ipi_mask_val;
> 	};
> };
> #else
> struct task_ipi_mask { };
> #endif
> 
> struct task_struct {
> 	...
>          struct task_ipi_mask			__private ipi_mask;
> 
> and have the magic accessors use
> 
>           ACCESS_PRIVATE(t->ipi_mask.member);
> 
> That allows static analysis to catch any incidential direct references.
> 

Agreed. Hiding the storage behind a private wrapper makes the intended
access pattern much clearer and lets sparse catch accidental direct
references.

I'll change this to use a struct task_ipi_mask __private field in
task_struct, just as you suggested, and access the union members only
through ACCESS_PRIVATE() in the helper functions.

I'll also fix the wording in changelog.

Thanks,
Chuyi


> Thanks,
> 
>          tglx