Re: [PATCH -next 2/4] cpuset: add helpers for cpuset related locks

[Chen Ridong <chenridong@huaweicloud.com>]

On 2025/8/9 23:56, Christophe JAILLET wrote:
> Le 08/08/2025 à 11:25, Chen Ridong a écrit :
>> From: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> Add guard_cpus_read_and_cpuset and guard_cpuset helpers for cpuset, which
>> will be user for subsequent patched to make code concise;
>>
>> Signed-off-by: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   include/linux/cpuset.h          |  1 +
>>   kernel/cgroup/cpuset-internal.h |  2 ++
>>   kernel/cgroup/cpuset.c          | 11 +++++++++++
>>   3 files changed, 14 insertions(+)
>>
>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>> index 2ddb256187b5..6153de28acf0 100644
>> --- a/include/linux/cpuset.h
>> +++ b/include/linux/cpuset.h
>> @@ -74,6 +74,7 @@ extern void inc_dl_tasks_cs(struct task_struct *task);
>>   extern void dec_dl_tasks_cs(struct task_struct *task);
>>   extern void cpuset_lock(void);
>>   extern void cpuset_unlock(void);
>> +extern void guard_cpuset(void);
>>   extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
>>   extern bool cpuset_cpus_allowed_fallback(struct task_struct *p);
>>   extern bool cpuset_cpu_is_isolated(int cpu);
>> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
>> index 75b3aef39231..084e19fe33d5 100644
>> --- a/kernel/cgroup/cpuset-internal.h
>> +++ b/kernel/cgroup/cpuset-internal.h
>> @@ -277,6 +277,8 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>>                       char *buf, size_t nbytes, loff_t off);
>>   int cpuset_common_seq_show(struct seq_file *sf, void *v);
>>   +void guard_cpus_read_and_cpuset(void);
>> +
>>   /*
>>    * cpuset-v1.c
>>    */
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index cf7cd2255265..f6cdb5cdffe8 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -260,6 +260,17 @@ void cpuset_unlock(void)
>>       mutex_unlock(&cpuset_mutex);
>>   }
>>   +void guard_cpuset(void)
>> +{
>> +    guard(mutex)(&cpuset_mutex);
>> +}
>> +
>> +void guard_cpus_read_and_cpuset(void)
>> +{
>> +    guard(cpus_read_lock)();
>> +    guard(mutex)(&cpuset_mutex);
>> +}
>> +
> 
> Not sure that it works like that.
> 
> I think that these 2 functions are just no-op because whatever is "garded", it will be release when
> the function exits.
> 
> So, if correct, all this serie does is removing some existing synchronisation mechanism.
> 
> Do I miss something obvious?
> 
> CJ
> 

Thank you for catching this issue. You're absolutely right - I made a critical mistake in the guard
function implementation.

After further investigation, I realized that when I ran the self-tests with CONFIG_LOCKDEP disabled,
I missed the lock protection warnings that would have revealed this problem earlier. Had Lockdep
been enabled, it would have immediately flagged the incorrect locking behavior.

Please ignore this patch series, Thanks.

Best regards,
Ridong