From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dggsgout11.his.huawei.com (dggsgout11.his.huawei.com [45.249.212.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 62EC028DB56; Sat, 31 Jan 2026 02:53:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769828018; cv=none; b=RD36lQeP5dahVcK4YohNio54dTKf9h7Ra9omNRt9SXuj94s7nc0xBty+1VGm3YRDNA57o6/dY8bKa/Ix/zWeRb9ra49S1ozWiP0ziGABNb6/uQWr/Zng51B3NnDBB8RHhDPHFsB/kmFMA/sjmz0NhqTtqTb6EjKB8zgkn6R0mNo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769828018; c=relaxed/simple; bh=ckDdNHinKayNaAQjF+Dt8pyywck5Qw0i3rEtQqWOKaE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=eSEVM9sLrXUJV3mSCl0kZnUBDuCTVtM3NmVyAcliPWNkNP6oOnraxzFjLmzzlHoBPJmFSEuo1cf93Cus8IHvqnvgiCV0n2/UHc7Wfb4SV57YTl7QTn/DEbMbkskfZfFelxP+P/5FOgPJKUARxRdgLLPzEk54Mnq5GyzgiofVDHs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com; spf=pass smtp.mailfrom=huaweicloud.com; arc=none smtp.client-ip=45.249.212.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=huaweicloud.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huaweicloud.com Received: from mail.maildlp.com (unknown [172.19.163.170]) by dggsgout11.his.huawei.com (SkyGuard) with ESMTPS id 4f2y9G1bHKzYQtgP; Sat, 31 Jan 2026 10:52:50 +0800 (CST) Received: from mail02.huawei.com (unknown [10.116.40.128]) by mail.maildlp.com (Postfix) with ESMTP id D39364056D; Sat, 31 Jan 2026 10:53:31 +0800 (CST) Received: from [10.67.111.176] (unknown [10.67.111.176]) by APP4 (Coremail) with SMTP id gCh0CgBH9fWpbn1p0lDjFg--.12988S2; Sat, 31 Jan 2026 10:53:31 +0800 (CST) Message-ID: <62022397-287c-4046-94de-058ff87ad728@huaweicloud.com> Date: Sat, 31 Jan 2026 10:53:28 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH/for-next v2 2/2] cgroup/cpuset: Introduce a new top level cpuset_top_mutex To: Waiman Long , Tejun Heo , Johannes Weiner , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Anna-Maria Behnsen , Frederic Weisbecker , Thomas Gleixner , Shuah Khan Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org References: <20260130154254.1422113-1-longman@redhat.com> <20260130154254.1422113-3-longman@redhat.com> Content-Language: en-US From: Chen Ridong In-Reply-To: <20260130154254.1422113-3-longman@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CM-TRANSID:gCh0CgBH9fWpbn1p0lDjFg--.12988S2 X-Coremail-Antispam: 1UD129KBjvAXoW3ur1fXFW8Gr1rJw4kCF1UAwb_yoW8Jry7to WSg395Cr1rJw1UCFZ8Ar1DAFW8Gw4ktr4xAr429rnrWF43ZFy7Ka43G3y2vrW3Ww4YkF4j y34aqrWvy39rtF1Un29KB7ZKAUJUUUU8529EdanIXcx71UUUUU7v73VFW2AGmfu7bjvjm3 AaLaJ3UjIYCTnIWjp_UUUYc7kC6x804xWl14x267AKxVW5JVWrJwAFc2x0x2IEx4CE42xK 8VAvwI8IcIk0rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4 AK67xGY2AK021l84ACjcxK6xIIjxv20xvE14v26F1j6w1UM28EF7xvwVC0I7IYx2IY6xkF 7I0E14v26r4UJVWxJr1l84ACjcxK6I8E87Iv67AKxVW0oVCq3wA2z4x0Y4vEx4A2jsIEc7 CjxVAFwI0_GcCE3s1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqx4xG64xvF2IEw4CE5I8C rVC2j2WlYx0E2Ix0cI8IcVAFwI0_Jr0_Jr4lYx0Ex4A2jsIE14v26r1j6r4UMcvjeVCFs4 IE7xkEbVWUJVW8JwACjcxG0xvEwIxGrwACI402YVCY1x02628vn2kIc2xKxwCY1x0262kK e7AKxVW8ZVWrXwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c 02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_GFv_ WrylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7 CjxVAFwI0_Gr0_Cr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v2 6r1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Gr1j6F4UJbIYCTnIWIevJa73UjIFyTuYvj xUIa0PDUUUU X-CM-SenderInfo: hfkh02xlgr0w46kxt4xhlfz01xgou0bp/ On 2026/1/30 23:42, Waiman Long wrote: > The current cpuset partition code is able to dynamically update > the sched domains of a running system and the corresponding > HK_TYPE_DOMAIN housekeeping cpumask to perform what is essentally the > "isolcpus=domain,..." boot command line feature at run time. > > The housekeeping cpumask update requires flushing a number of different > workqueues which may not be safe with cpus_read_lock() held as the > workqueue flushing code may acquire cpus_read_lock() or acquiring locks > which have locking dependency with cpus_read_lock() down the chain. Below > is an example of such circular locking problem. > > ====================================================== > WARNING: possible circular locking dependency detected > 6.18.0-test+ #2 Tainted: G S > ------------------------------------------------------ > test_cpuset_prs/10971 is trying to acquire lock: > ffff888112ba4958 ((wq_completion)sync_wq){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x7a/0x180 > > but task is already holding lock: > ffffffffae47f450 (cpuset_mutex){+.+.}-{4:4}, at: cpuset_partition_write+0x85/0x130 > > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > -> #4 (cpuset_mutex){+.+.}-{4:4}: > -> #3 (cpu_hotplug_lock){++++}-{0:0}: > -> #2 (rtnl_mutex){+.+.}-{4:4}: > -> #1 ((work_completion)(&arg.work)){+.+.}-{0:0}: > -> #0 ((wq_completion)sync_wq){+.+.}-{0:0}: > > Chain exists of: > (wq_completion)sync_wq --> cpu_hotplug_lock --> cpuset_mutex > > 5 locks held by test_cpuset_prs/10971: > #0: ffff88816810e440 (sb_writers#7){.+.+}-{0:0}, at: ksys_write+0xf9/0x1d0 > #1: ffff8891ab620890 (&of->mutex#2){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x260/0x5f0 > #2: ffff8890a78b83e8 (kn->active#187){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x2b6/0x5f0 > #3: ffffffffadf32900 (cpu_hotplug_lock){++++}-{0:0}, at: cpuset_partition_write+0x77/0x130 > #4: ffffffffae47f450 (cpuset_mutex){+.+.}-{4:4}, at: cpuset_partition_write+0x85/0x130 > > Call Trace: > > : > touch_wq_lockdep_map+0x93/0x180 > __flush_workqueue+0x111/0x10b0 > housekeeping_update+0x12d/0x2d0 > update_parent_effective_cpumask+0x595/0x2440 > update_prstate+0x89d/0xce0 > cpuset_partition_write+0xc5/0x130 > cgroup_file_write+0x1a5/0x680 > kernfs_fop_write_iter+0x3df/0x5f0 > vfs_write+0x525/0xfd0 > ksys_write+0xf9/0x1d0 > do_syscall_64+0x95/0x520 > entry_SYSCALL_64_after_hwframe+0x76/0x7e > > To avoid such a circular locking dependency problem, we have to > call housekeeping_update() without holding the cpus_read_lock() and > cpuset_mutex. The current set of wq's flushed by housekeeping_update() > may not have work functions that call cpus_read_lock() directly, > but we are likely to extend the list of wq's that are flushed in the > future. Moreover, the current set of work functions may hold locks that > may have cpu_hotplug_lock down the dependency chain. > > One way to do that is to introduce a new top level cpuset_top_mutex > which will be acquired first. This new cpuset_top_mutex will provide > the need mutual exclusion without the need to hold cpus_read_lock(). > Introducing a new global lock warrants careful consideration. I wonder if we could make all updates to isolated_cpus asynchronous. If that is feasible, we could avoid adding a global lock altogether. If not, we need to clarify which updates must remain synchronous and which ones can be handled asynchronously. > As cpus_read_lock() is now no longer held when > tmigr_isolated_exclude_cpumask() is called, it needs to acquire it > directly. > > The lockdep_is_cpuset_held() is also updated to check the new > cpuset_top_mutex. > > Signed-off-by: Waiman Long > --- > kernel/cgroup/cpuset.c | 101 +++++++++++++++++++++++----------- > kernel/sched/isolation.c | 4 +- > kernel/time/timer_migration.c | 3 +- > 3 files changed, 70 insertions(+), 38 deletions(-) > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index 0b0eb1df09d5..edccfa2df9da 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -78,13 +78,13 @@ static cpumask_var_t subpartitions_cpus; > static cpumask_var_t isolated_cpus; > > /* > - * isolated_cpus updating flag (protected by cpuset_mutex) > + * isolated_cpus updating flag (protected by cpuset_top_mutex) > * Set if isolated_cpus is going to be updated in the current > * cpuset_mutex crtical section. > */ > static bool isolated_cpus_updating; > > -/* Both cpuset_mutex and cpus_read_locked acquired */ > +/* cpuset_top_mutex acquired */ > static bool cpuset_locked; > > /* > @@ -222,29 +222,44 @@ struct cpuset top_cpuset = { > }; > > /* > - * There are two global locks guarding cpuset structures - cpuset_mutex and > - * callback_lock. The cpuset code uses only cpuset_mutex. Other kernel > - * subsystems can use cpuset_lock()/cpuset_unlock() to prevent change to cpuset > - * structures. Note that cpuset_mutex needs to be a mutex as it is used in > - * paths that rely on priority inheritance (e.g. scheduler - on RT) for > - * correctness. > + * CPUSET Locking Convention > + * ------------------------- > * > - * A task must hold both locks to modify cpusets. If a task holds > - * cpuset_mutex, it blocks others, ensuring that it is the only task able to > - * also acquire callback_lock and be able to modify cpusets. It can perform > - * various checks on the cpuset structure first, knowing nothing will change. > - * It can also allocate memory while just holding cpuset_mutex. While it is > - * performing these checks, various callback routines can briefly acquire > - * callback_lock to query cpusets. Once it is ready to make the changes, it > - * takes callback_lock, blocking everyone else. > + * Below are the four global locks guarding cpuset structures in lock > + * acquisition order: > + * - cpuset_top_mutex > + * - cpu_hotplug_lock (cpus_read_lock/cpus_write_lock) > + * - cpuset_mutex > + * - callback_lock (raw spinlock) > * > - * Calls to the kernel memory allocator can not be made while holding > - * callback_lock, as that would risk double tripping on callback_lock > - * from one of the callbacks into the cpuset code from within > - * __alloc_pages(). > + * The first cpuset_top_mutex will be held except when calling into > + * cpuset_handle_hotplug() from the CPU hotplug code where cpus_write_lock > + * and cpuset_mutex will be held instead. > * > - * If a task is only holding callback_lock, then it has read-only > - * access to cpusets. > + * As cpuset will now indirectly flush a number of different workqueues in > + * housekeeping_update() when the set of isolated CPUs is going to be changed, > + * it may not be safe from the circular locking perspective to hold the > + * cpus_read_lock. So cpus_read_lock and cpuset_mutex will be released before > + * calling housekeeping_update() and re-acquired afterward. > + * > + * A task must hold all the remaining three locks to modify externally visible > + * or used fields of cpusets, though some of the internally used cpuset fields > + * can be modified without holding callback_lock. If only reliable read access > + * of the externally used fields are needed, a task can hold either > + * cpuset_mutex or callback_lock which are exposed to other subsystems. > + * > + * If a task holds cpu_hotplug_lock and cpuset_mutex, it blocks others, > + * ensuring that it is the only task able to also acquire callback_lock and > + * be able to modify cpusets. It can perform various checks on the cpuset > + * structure first, knowing nothing will change. It can also allocate memory > + * without holding callback_lock. While it is performing these checks, various > + * callback routines can briefly acquire callback_lock to query cpusets. Once > + * it is ready to make the changes, it takes callback_lock, blocking everyone > + * else. > + * > + * Calls to the kernel memory allocator cannot be made while holding > + * callback_lock which is a spinlock, as the memory allocator may sleep or > + * call back into cpuset code and acquire callback_lock. > * > * Now, the task_struct fields mems_allowed and mempolicy may be changed > * by other task, we use alloc_lock in the task_struct fields to protect > @@ -255,6 +270,7 @@ struct cpuset top_cpuset = { > * cpumasks and nodemasks. > */ > > +static DEFINE_MUTEX(cpuset_top_mutex); > static DEFINE_MUTEX(cpuset_mutex); > > /** > @@ -278,6 +294,18 @@ void lockdep_assert_cpuset_lock_held(void) > lockdep_assert_held(&cpuset_mutex); > } > > +static void cpuset_partial_lock(void) > +{ > + cpus_read_lock(); > + mutex_lock(&cpuset_mutex); > +} > + > +static void cpuset_partial_unlock(void) > +{ > + mutex_unlock(&cpuset_mutex); > + cpus_read_unlock(); > +} > + > /** > * cpuset_full_lock - Acquire full protection for cpuset modification > * > @@ -286,22 +314,22 @@ void lockdep_assert_cpuset_lock_held(void) > */ > void cpuset_full_lock(void) > { > - cpus_read_lock(); > - mutex_lock(&cpuset_mutex); > + mutex_lock(&cpuset_top_mutex); > + cpuset_partial_lock(); > cpuset_locked = true; > } > > void cpuset_full_unlock(void) > { > cpuset_locked = false; > - mutex_unlock(&cpuset_mutex); > - cpus_read_unlock(); > + cpuset_partial_unlock(); > + mutex_unlock(&cpuset_top_mutex); > } > > #ifdef CONFIG_LOCKDEP > bool lockdep_is_cpuset_held(void) > { > - return lockdep_is_held(&cpuset_mutex); > + return lockdep_is_held(&cpuset_top_mutex); > } > #endif > > @@ -1292,12 +1320,12 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus) > > static void isolcpus_workfn(struct work_struct *work) > { > - cpuset_full_lock(); > - if (isolated_cpus_updating) { > - WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0); > - isolated_cpus_updating = false; > - } > - cpuset_full_unlock(); > + guard(mutex)(&cpuset_top_mutex); > + if (!isolated_cpus_updating) > + return; > + > + WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0); > + isolated_cpus_updating = false; > } > > /* > @@ -1331,8 +1359,15 @@ static void update_isolation_cpumasks(void) > return; > } > > + lockdep_assert_held(&cpuset_top_mutex); > + /* > + * Release cpus_read_lock & cpuset_mutex before calling > + * housekeeping_update() and re-acquiring them afterward. > + */ > + cpuset_partial_unlock(); > WARN_ON_ONCE(housekeeping_update(isolated_cpus) < 0); > isolated_cpus_updating = false; > + cpuset_partial_lock(); > } > > /** > diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c > index 3b725d39c06e..ef152d401fe2 100644 > --- a/kernel/sched/isolation.c > +++ b/kernel/sched/isolation.c > @@ -123,8 +123,6 @@ int housekeeping_update(struct cpumask *isol_mask) > struct cpumask *trial, *old = NULL; > int err; > > - lockdep_assert_cpus_held(); > - > trial = kmalloc(cpumask_size(), GFP_KERNEL); > if (!trial) > return -ENOMEM; > @@ -136,7 +134,7 @@ int housekeeping_update(struct cpumask *isol_mask) > } > > if (!housekeeping.flags) > - static_branch_enable_cpuslocked(&housekeeping_overridden); > + static_branch_enable(&housekeeping_overridden); > > if (housekeeping.flags & HK_FLAG_DOMAIN) > old = housekeeping_cpumask_dereference(HK_TYPE_DOMAIN); > diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c > index 6da9cd562b20..244a8d025e78 100644 > --- a/kernel/time/timer_migration.c > +++ b/kernel/time/timer_migration.c > @@ -1559,8 +1559,6 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask) > cpumask_var_t cpumask __free(free_cpumask_var) = CPUMASK_VAR_NULL; > int cpu; > > - lockdep_assert_cpus_held(); > - > if (!works) > return -ENOMEM; > if (!alloc_cpumask_var(&cpumask, GFP_KERNEL)) > @@ -1570,6 +1568,7 @@ int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask) > * First set previously isolated CPUs as available (unisolate). > * This cpumask contains only CPUs that switched to available now. > */ > + guard(cpus_read_lock)(); > cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask); > cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask); > -- Best regards, Ridong