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
log:
[ 43.853794] ------------[ cut here ]------------
[ 43.853798] WARNING: CPU: 7 PID: 463 at ../kernel/cgroup/cpuset.c:2279 cpuset_attach+0xee/0x1f0
[ 43.853806] Modules linked in:
[ 43.853807] CPU: 7 PID: 463 Comm: bash Not tainted 5.16.0-rc4+ #10
[ 43.853810] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
[ 43.853811] RIP: 0010:cpuset_attach+0xee/0x1f0
[ 43.853814] Code: ff ff 48 85 c0 48 89 c3 74 24 48 81 fd 40 42 54 82 75 96 80 bb 38 07 00 00 6f 48 8b 05 93 b3 55 01 48 89 05 bc 05 bb 01 75 97 <0f> 0b eb b3 48 8b 85 e8 00 00 00 48 85
[ 43.853816] RSP: 0018:ffffc90000623c30 EFLAGS: 00010246
[ 43.853818] RAX: 0000000000000000 RBX: ffff888101f39c80 RCX: 0000000000000001
[ 43.853819] RDX: 0000000000007fff RSI: ffffffff82cd5708 RDI: ffff888101f39c80
[ 43.853821] RBP: ffff8881001afe00 R08: 0000000000000000 R09: ffffc90000623d00
[ 43.853822] R10: ffffc900000a3de8 R11: 0000000000000001 R12: ffffc90000623cf0
[ 43.853823] R13: ffffffff82cd56d0 R14: ffffffff82544240 R15: 0000000000000001
[ 43.853824] FS: 00007f012414d740(0000) GS:ffff8882b5bc0000(0000) knlGS:0000000000000000
[ 43.853828] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 43.853829] CR2: 000055cfdb27de28 CR3: 00000001020cc000 CR4: 00000000000006e0
[ 43.853830] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 43.853831] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 43.853832] Call Trace:
[ 43.853846] <TASK>
[ 43.853848] cgroup_migrate_execute+0x319/0x410
[ 43.853853] cgroup_attach_task+0x159/0x200
[ 43.853857] ? __cgroup1_procs_write.constprop.21+0x10d/0x170
[ 43.853858] __cgroup1_procs_write.constprop.21+0x10d/0x170
[ 43.853860] cgroup_file_write+0x65/0x160
[ 43.853863] kernfs_fop_write_iter+0x12a/0x1a0
[ 43.853870] new_sync_write+0x11d/0x1b0
[ 43.853877] vfs_write+0x232/0x290
[ 43.853880] ksys_write+0x9c/0xd0
[ 43.853882] ? fpregs_assert_state_consistent+0x19/0x40
[ 43.853886] do_syscall_64+0x3a/0x80
[ 43.853896] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 43.853902] RIP: 0033:0x7f012381f224
[ 43.853904] Code: 89 02 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 8d 05 c1 07 2e 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 f3 c3 66 90 45
[ 43.853906] RSP: 002b:00007ffd3f411f28 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 43.853908] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007f012381f224
[ 43.853909] RDX: 0000000000000004 RSI: 000055cfdb297a70 RDI: 0000000000000001
[ 43.853910] RBP: 000055cfdb297a70 R08: 000000000000000a R09: 0000000000000003
[ 43.853911] R10: 000000000000000a R11: 0000000000000246 R12: 00007f0123afb760
[ 43.853913] R13: 0000000000000004 R14: 00007f0123af72a0 R15: 00007f0123af6760
[ 43.853914] </TASK>
[ 43.853915] ---[ end trace 8292bcee7ea90403 ]---
On Fri, Jan 14, 2022 at 09:15:06AM +0800, Zhang Qiao <zhangqiao22@huawei.com> wrote:
> 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.
Do you have more information on what hotplug event and what error
(from set_cpus_allowed_ptr() you observe? (And what's src/dst cpuset wrt
root/non-root)?
> Can we use cpus_read_lock()/cpus_read_unlock() to guarantee that set_cpus_allowed_ptr()
> doesn't fail, as follows:
I'm wondering what can be wrong with the current actors:
cpuset_can_attach
down_read(cpuset_rwsem)
// check all migratees
up_read(cpuset_rwsem)
[ _cpu_down / cpuhp_setup_state ]
schedule_work
...
cpuset_hotplug_update_tasks
down_write(cpuset_rwsem)
up_write(cpuset_rwsem)
... flush_work
[ _cpu_down / cpu_up_down_serialize_trainwrecks ]
cpuset_attach
down_write(cpuset_rwsem)
set_cpus_allowed_ptr(allowed_cpus_weird)
up_write(cpuset_rwsem)
The statement in cpuset_attach() about cpuset_can_attach() test is not
so strong since task_can_attach() is mostly a pass for non-deadline
tasks. Still, the use of cpuset_rwsem above should synchronize (I may be
mistaken) the changes of cpuset's cpu masks, so I'd be interested about
the details above to understand why the current approach doesn't work.
The additional cpus_read_{,un}lock (when reordered wrt cpuset_rwsem)
may work but your patch should explain why (in what situation).
My .02€,
Michal
hello
在 2022/1/19 21:02, Michal Koutný 写道:
> On Fri, Jan 14, 2022 at 09:15:06AM +0800, Zhang Qiao <zhangqiao22@huawei.com> wrote:
>> 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.
>
> Do you have more information on what hotplug event and what error
> (from set_cpus_allowed_ptr() you observe? (And what's src/dst cpuset wrt
> root/non-root)?
I ran the LTP testcases and a test scripts that do hotplug on a random cpu at the same time.
The race condition quickly, and I can't reproduce it so far.
By reading code about set_cpus_allowed_ptr(), i think __set_cpus_allowed_ptr_locked() will
be failed when new_mask and cpu_active_mask do not intersect, as follows:
__set_cpus_allowed_ptr_locked():
....
const struct cpumask *cpu_valid_mask = cpu_active_mask;
dest_cpu = cpumask_any_and_distribute(cpu_valid_mask, new_mask);
if (dest_cpu >= nr_cpu_ids) {
ret = -EINVAL;
goto out;
}
....
}
>
>> Can we use cpus_read_lock()/cpus_read_unlock() to guarantee that set_cpus_allowed_ptr()
>> doesn't fail, as follows:
>
> I'm wondering what can be wrong with the current actors:
>
> cpuset_can_attach
> down_read(cpuset_rwsem)
> // check all migratees
> up_read(cpuset_rwsem)
> [ _cpu_down / cpuhp_setup_state ]
> schedule_work
> ...
> cpuset_hotplug_update_tasks
> down_write(cpuset_rwsem)
> up_write(cpuset_rwsem)
> ... flush_work
> [ _cpu_down / cpu_up_down_serialize_trainwrecks ]
> cpuset_attach
> down_write(cpuset_rwsem)
> set_cpus_allowed_ptr(allowed_cpus_weird)
> up_write(cpuset_rwsem)
>
i think the troublesome scenario as follows:
cpuset_can_attach
down_read(cpuset_rwsem)
// check all migratees
up_read(cpuset_rwsem)
[ _cpu_down / cpuhp_setup_state ]
cpuset_attach
down_write(cpuset_rwsem)
guarantee_online_cpus() // (load cpus_attach)
sched_cpu_deactivate
set_cpu_active(cpu, false) // will change cpu_active_mask
set_cpus_allowed_ptr(cpus_attach)
__set_cpus_allowed_ptr_locked()
// (if the intersection of cpus_attach and
cpu_active_mask is empty, will return -EINVAL)
up_write(cpuset_rwsem)
schedule_work
...
cpuset_hotplug_update_tasks
down_write(cpuset_rwsem)
up_write(cpuset_rwsem)
... flush_work
[ _cpu_down / cpu_up_down_serialize_trainwrecks ]
Regards,
Qiao
> The statement in cpuset_attach() about cpuset_can_attach() test is not
> so strong since task_can_attach() is mostly a pass for non-deadline
> tasks. Still, the use of cpuset_rwsem above should synchronize (I may be
> mistaken) the changes of cpuset's cpu masks, so I'd be interested about
> the details above to understand why the current approach doesn't work.
>
> The additional cpus_read_{,un}lock (when reordered wrt cpuset_rwsem)
> may work but your patch should explain why (in what situation).
>
> My .02€,
> Michal
> .
>
On Thu, Jan 20, 2022 at 03:14:22PM +0800, Zhang Qiao <zhangqiao22@huawei.com> wrote: > i think the troublesome scenario as follows: > cpuset_can_attach > down_read(cpuset_rwsem) > // check all migratees > up_read(cpuset_rwsem) > [ _cpu_down / cpuhp_setup_state ] > cpuset_attach > down_write(cpuset_rwsem) > guarantee_online_cpus() // (load cpus_attach) > sched_cpu_deactivate > set_cpu_active(cpu, false) // will change cpu_active_mask > set_cpus_allowed_ptr(cpus_attach) > __set_cpus_allowed_ptr_locked() > // (if the intersection of cpus_attach and > cpu_active_mask is empty, will return -EINVAL) > up_write(cpuset_rwsem) > schedule_work > ... > cpuset_hotplug_update_tasks > down_write(cpuset_rwsem) > up_write(cpuset_rwsem) > ... flush_work > [ _cpu_down / cpu_up_down_serialize_trainwrecks ] Thanks, a locking loophole indeed. FTR, meanwhile I noticed: a) cpuset_fork() looks buggy when CLONE_INTO_CGROUP (and dst.cpus != src.cpus), b) it'd be affected with similar hotplug race. Michal
在 2022/1/20 22:02, Michal Koutný 写道: > On Thu, Jan 20, 2022 at 03:14:22PM +0800, Zhang Qiao <zhangqiao22@huawei.com> wrote: >> i think the troublesome scenario as follows: >> cpuset_can_attach >> down_read(cpuset_rwsem) >> // check all migratees >> up_read(cpuset_rwsem) >> [ _cpu_down / cpuhp_setup_state ] >> cpuset_attach >> down_write(cpuset_rwsem) >> guarantee_online_cpus() // (load cpus_attach) >> sched_cpu_deactivate >> set_cpu_active(cpu, false) // will change cpu_active_mask >> set_cpus_allowed_ptr(cpus_attach) >> __set_cpus_allowed_ptr_locked() >> // (if the intersection of cpus_attach and >> cpu_active_mask is empty, will return -EINVAL) >> up_write(cpuset_rwsem) >> schedule_work >> ... >> cpuset_hotplug_update_tasks >> down_write(cpuset_rwsem) >> up_write(cpuset_rwsem) >> ... flush_work >> [ _cpu_down / cpu_up_down_serialize_trainwrecks ] > > Thanks, a locking loophole indeed. > > FTR, meanwhile I noticed: a) cpuset_fork() looks buggy when > CLONE_INTO_CGROUP (and dst.cpus != src.cpus), b) it'd be affected with > similar hotplug race. Yes, it shouldn't set the current task's cpumak to the child process at cpuset_fork(). Regards, Qiao . > > Michal > . >
© 2016 - 2026 Red Hat, Inc.