lkml.org 
[lkml]   [2022]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Question] set_cpus_allowed_ptr() call failed at cpuset_attach()
From
On 1/16/22 21:25, Zhang Qiao wrote:
> hello
>
> 在 2022/1/15 4:33, Waiman Long 写道:
>> On 1/14/22 11:20, Tejun Heo wrote:
>>> (cc'ing Waiman and Michal and quoting whole body)
>>>
>>> Seems sane to me but let's hear what Waiman and Michal think.
>>>
>>> On Fri, Jan 14, 2022 at 09:15:06AM +0800, Zhang Qiao wrote:
>>>> Hello everyone
>>>>
>>>>     I found the following warning log on qemu. I migrated a task from one cpuset cgroup to
>>>> another, while I also performed the cpu hotplug operation, and got following calltrace.
>>>>
>>>>     This may lead to a inconsistency between the affinity of the task and cpuset.cpus of the
>>>> dest cpuset, but this task can be successfully migrated to the dest cpuset cgroup.
>>>>
>>>>     Can we use cpus_read_lock()/cpus_read_unlock() to guarantee that set_cpus_allowed_ptr()
>>>> doesn't fail, as follows:
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index d0e163a02099..2535d23d2c51 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -2265,6 +2265,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>>>          guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
>>>>
>>>>          cgroup_taskset_for_each(task, css, tset) {
>>>> +               cpus_read_lock();
>>>>                  if (cs != &top_cpuset)
>>>>                          guarantee_online_cpus(task, cpus_attach);
>>>>                  else
>>>> @@ -2274,6 +2275,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>>>>                   * fail.  TODO: have a better way to handle failure here
>>>>                   */
>>>>                  WARN_ON_ONCE(set_cpus_allowed_ptr(task, cpus_attach));
>>>> +               cpus_read_unlock();
>>>>
>>>>
>>>>     Is there a better solution?
>>>>
>>>>     Thanks
>> The change looks OK to me. However, we may need to run the full set of regression test to make sure that lockdep won't complain about potential deadlock.
>>
> I run the test with lockdep enabled, and got lockdep warning like that below.
> so we should take the cpu_hotplug_lock first, then take the cpuset_rwsem lock.
>
> thanks,
> Zhang Qiao
>
> [ 38.420372] ======================================================
> [ 38.421339] WARNING: possible circular locking dependency detected
> [ 38.422312] 5.16.0-rc4+ #13 Not tainted
> [ 38.422920] ------------------------------------------------------
> [ 38.423883] bash/594 is trying to acquire lock:
> [ 38.424595] ffffffff8286afc0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuset_attach+0xc2/0x1e0
> [ 38.425880]
> [ 38.425880] but task is already holding lock:
> [ 38.426787] ffffffff8296a5a0 (&cpuset_rwsem){++++}-{0:0}, at: cpuset_attach+0x3e/0x1e0
> [ 38.428015]
> [ 38.428015] which lock already depends on the new lock.
> [ 38.428015]
> [ 38.429279]
> [ 38.429279] the existing dependency chain (in reverse order) is:
> [ 38.430445]
> [ 38.430445] -> #1 (&cpuset_rwsem){++++}-{0:0}:
> [ 38.431371] percpu_down_write+0x42/0x130
> [ 38.432085] cpuset_css_online+0x2b/0x2e0
> [ 38.432808] online_css+0x24/0x80
> [ 38.433411] cgroup_apply_control_enable+0x2fa/0x330
> [ 38.434273] cgroup_mkdir+0x396/0x4c0
> [ 38.434930] kernfs_iop_mkdir+0x56/0x80
> [ 38.435614] vfs_mkdir+0xde/0x190
> [ 38.436220] do_mkdirat+0x7d/0xf0
> [ 38.436824] __x64_sys_mkdir+0x21/0x30
> [ 38.437495] do_syscall_64+0x3a/0x80
> [ 38.438145] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 38.439015]
> [ 38.439015] -> #0 (cpu_hotplug_lock){++++}-{0:0}:
> [ 38.439980] __lock_acquire+0x17f6/0x2260
> [ 38.440691] lock_acquire+0x277/0x320
> [ 38.441347] cpus_read_lock+0x37/0xc0
> [ 38.442011] cpuset_attach+0xc2/0x1e0
> [ 38.442671] cgroup_migrate_execute+0x3a6/0x490
> [ 38.443461] cgroup_attach_task+0x22c/0x3d0
> [ 38.444197] __cgroup1_procs_write.constprop.21+0x10d/0x170
> [ 38.445145] cgroup_file_write+0x6f/0x230
> [ 38.445860] kernfs_fop_write_iter+0x130/0x1b0
> [ 38.446636] new_sync_write+0x120/0x1b0
> [ 38.447319] vfs_write+0x359/0x3b0
> [ 38.447937] ksys_write+0xa2/0xe0
> [ 38.448540] do_syscall_64+0x3a/0x80
> [ 38.449183] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 38.450057]
> [ 38.450057] other info that might help us debug this:
> [ 38.450057]
> [ 38.451297] Possible unsafe locking scenario:
> [ 38.451297]
> [ 38.452218] CPU0 CPU1
> [ 38.452935] ---- ----
> [ 38.453650] lock(&cpuset_rwsem);
> [ 38.454188] lock(cpu_hotplug_lock);
> [ 38.455148] lock(&cpuset_rwsem);
> [ 38.456069] lock(cpu_hotplug_lock);

Yes, you need to play around with lock ordering to make sure that
lockdep won't complain.

Cheers,
Longman

\
 
 \ /
  Last update: 2022-01-17 05:36    [W:0.069 / U:1.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site