linux-kernel - Re: [PATCH -next v5 3/3] cpuset: add helpers for cpus read and cpuset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2b574bb7-0192-4a91-8925-bd4c6cc8a407@huaweicloud.com>
Date: Wed, 27 Aug 2025 14:23:17 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, Michal Koutný
 <mkoutny@...e.com>
Cc: tj@...nel.org, hannes@...xchg.org, cgroups@...r.kernel.org,
 linux-kernel@...r.kernel.org, lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next v5 3/3] cpuset: add helpers for cpus read and
 cpuset_mutex locks



On 2025/8/26 22:43, Waiman Long wrote:
> 
> On 8/26/25 10:23 AM, Michal Koutný wrote:
>> (I wrote this yesterday before merging but I'm still sending it to give
>> my opinion ;-))
>>
>> On Mon, Aug 25, 2025 at 03:23:52AM +0000, Chen Ridong <chenridong@...weicloud.com> wrote:
>>> From: Chen Ridong <chenridong@...wei.com>
>>>
>>> cpuset: add helpers for cpus_read_lock and cpuset_mutex locks.
>>>
>>> Replace repetitive locking patterns with new helpers:
>>> - cpuset_full_lock()
>>> - cpuset_full_unlock()
>> I don't see many precedents elsewhere in the kernel for such naming
>> (like _lock and _full_lock()). Wouldn't it be more illustrative to have
>> cpuset_read_lock() and cpuset_write_lock()? (As I'm looking at current
>> users and your accompanying comments which could be substituted with
>> the more conventional naming.)
> Good naming is always an issue. Using cpuset_read_lock/cpuset_write_lock will be more confusing as
> the current locking scheme is exclusive.
>> (Also if you decide going this direction, please mention commit
>> 111cd11bbc548 ("sched/cpuset: Bring back cpuset_mutex") in the message
>> so that it doesn't tempt to do further changes.)
>>
>>
>>> This makes the code cleaner and ensures consistent lock ordering.
>> Lock guards anyone? (When you're touching this and seeking clean code.)
> 
> Yes, I guess we can use lock guards here. You are welcome to send a patch to do that.
> 

I attempted to define the cpuset_full_lock() macro, but the initial implementation was inconsistent
with our coding conventions.
Initial version:

#define cpuset_full_lock() \
  guard(cpus_read_lock)(); \
  guard(mutex)(&cpuset_mutex);

It was suggested to use a do-while construct for proper scoping. but it could not work if we define as:

#define cpuset_full_lock() \
 do { 			   \
  guard(cpus_read_lock)(); \
  guard(mutex)(&cpuset_mutex); \
 } while(0)

So I sent this patch version.

-- 
Best regards,
Ridong