Messages in this thread Patch in this message | | | Date | Wed, 23 Nov 2022 15:23:55 -0500 | Subject | Re: [PATCH] cgroup/cpuset: Optimize update_tasks_nodemask() | From | Waiman Long <> |
| |
On 11/23/22 03:21, haifeng.xu wrote: > When change the 'cpuset.mems' under some cgroup, system will hung > for a long time. From the dmesg, many processes or theads are > stuck in fork/exit. The reason is show as follows. > > thread A: > cpuset_write_resmask /* takes cpuset_rwsem */ > ... > update_tasks_nodemask > mpol_rebind_mm /* waits mmap_lock */ > > thread B: > worker_thread > ... > cpuset_migrate_mm_workfn > do_migrate_pages /* takes mmap_lock */ > > thread C: > cgroup_procs_write /* takes cgroup_mutex and cgroup_threadgroup_rwsem */ > ... > cpuset_can_attach > percpu_down_write /* waits cpuset_rwsem */ > > Once update the nodemasks of cpuset, thread A wakes up thread B to > migrate mm. But when thread A iterates through all tasks, including > child threads and group leader, it has to wait the mmap_lock which > has been take by thread B. Unfortunately, thread C wants to migrate > tasks into cgroup at this moment, it must wait thread A to release > cpuset_rwsem. If thread B spends much time to migrate mm, the > fork/exit which acquire cgroup_threadgroup_rwsem also need to > wait for a long time. > > There is no need to migrate the mm of child threads which is > shared with group leader. Just iterate through the group > leader only. > > Signed-off-by: haifeng.xu <haifeng.xu@shopee.com> > --- > kernel/cgroup/cpuset.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index 589827ccda8b..43cbd09546d0 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -1968,6 +1968,9 @@ static void update_tasks_nodemask(struct cpuset *cs) > > cpuset_change_task_nodemask(task, &newmems); > > + if (!thread_group_leader(task)) > + continue; > + > mm = get_task_mm(task); > if (!mm) > continue;
Could you try the attached test patch to see if it can fix your problem? Something along the line of this patch will be more acceptable.
Thanks, Longman
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index b474289c15b8..9c17b6d4877c 100644 --- a/kernel/cgroup/cpuset.c +++ b/kernel/cgroup/cpuset.c @@ -1942,6 +1942,7 @@ static void update_tasks_nodemask(struct cpuset *cs) static nodemask_t newmems; /* protected by cpuset_rwsem */ struct css_task_iter it; struct task_struct *task; + bool migrate; cpuset_being_rebound = cs; /* causes mpol_dup() rebind */ @@ -1957,19 +1958,25 @@ static void update_tasks_nodemask(struct cpuset *cs) * It's ok if we rebind the same mm twice; mpol_rebind_mm() * is idempotent. Also migrate pages in each mm to new nodes. */ + migrate = is_memory_migrate(cs); css_task_iter_start(&cs->css, 0, &it); while ((task = css_task_iter_next(&it))) { struct mm_struct *mm; - bool migrate; cpuset_change_task_nodemask(task, &newmems); + /* + * Skip mm update if a non group leader task and its group + * leader are in the same cpuset. + */ + if (!thread_group_leader(task) && + (task_cs(task->group_leader) == cs)) + continue; + mm = get_task_mm(task); if (!mm) continue; - migrate = is_memory_migrate(cs); - mpol_rebind_mm(mm, &cs->mems_allowed); if (migrate) cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems); | |