From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753516AbdLMO2l (ORCPT ); Wed, 13 Dec 2017 09:28:41 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:46928 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753534AbdLMO2a (ORCPT ); Wed, 13 Dec 2017 09:28:30 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 72B44604BE Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=prsood@codeaurora.org Subject: Re: [PATCH] cgroup/cpuset: fix circular locking dependency To: Tejun Heo Cc: Peter Zijlstra , avagin@gmail.com, mingo@kernel.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, sramana@codeaurora.org References: <1511868946-23959-1-git-send-email-prsood@codeaurora.org> <623f214b-8b9a-f967-7a3d-ca9c06151267@codeaurora.org> <20171204202219.GF2421075@devbig577.frc2.facebook.com> <20171204225825.GP2421075@devbig577.frc2.facebook.com> <20171204230117.GF20227@worktop.programming.kicks-ass.net> <4e63b5e9-1696-910f-16ac-4d4d7eb98725@codeaurora.org> <40968aea-cd73-5ce4-d559-962d91e315c5@codeaurora.org> <20171211153258.GI2421075@devbig577.frc2.facebook.com> From: Prateek Sood Message-ID: Date: Wed, 13 Dec 2017 19:58:24 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20171211153258.GI2421075@devbig577.frc2.facebook.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/11/2017 09:02 PM, Tejun Heo wrote: > Hello, Prateek. > > On Fri, Dec 08, 2017 at 05:15:55PM +0530, Prateek Sood wrote: >> There is one deadlock issue during cgroup migration from cpu >> hotplug path when a task T is being moved from source to >> destination cgroup. >> >> kworker/0:0 >> cpuset_hotplug_workfn() >> cpuset_hotplug_update_tasks() >> hotplug_update_tasks_legacy() >> remove_tasks_in_empty_cpuset() >> cgroup_transfer_tasks() // stuck in iterator loop >> cgroup_migrate() >> cgroup_migrate_add_task() >> >> In cgroup_migrate_add_task() it checks for PF_EXITING flag of task T. >> Task T will not migrate to destination cgroup. css_task_iter_start() >> will keep pointing to task T in loop waiting for task T cg_list node >> to be removed. > > Heh, that's a bug in cgroup_transfer_tasks() which happened because I > forgot to update when we changed how we handle exiting tasks. The > right thing to do here is making cgroup_transfer_tasks() repeat iff > there were a valid migration target which didn't get transferred. > > Thanks. > Hi TJ, Did you mean something like below. If not then could you please share a patch for this problem in cgroup_transfer_tasks(). diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 473e0c0..41de618 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -143,6 +143,8 @@ struct task_struct *cgroup_taskset_next(struct cgroup_taskset *tset, void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags, struct css_task_iter *it); +void css_task_migrate_iter_start(struct cgroup_subsys_state *css, + unsigned int flags, struct css_task_iter *it); struct task_struct *css_task_iter_next(struct css_task_iter *it); void css_task_iter_end(struct css_task_iter *it); diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c index 024085d..12279ae 100644 --- a/kernel/cgroup/cgroup-v1.c +++ b/kernel/cgroup/cgroup-v1.c @@ -122,7 +122,7 @@ int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from) * ->can_attach() fails. */ do { - css_task_iter_start(&from->self, 0, &it); + css_task_migrate_iter_start(&from->self, 0, &it); task = css_task_iter_next(&it); if (task) get_task_struct(task); diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 0b1ffe1..3c1d2d2 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -4189,6 +4189,42 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags, spin_unlock_irq(&css_set_lock); } +void css_task_migrate_iter_start(struct cgroup_subsys_state *css, + unsigned int flags, struct css_task_iter *it) +{ + struct task_struct *task = NULL; + /* no one should try to iterate before mounting cgroups */ + WARN_ON_ONCE(!use_task_css_set_links); + + memset(it, 0, sizeof(*it)); + + spin_lock_irq(&css_set_lock); + + it->ss = css->ss; + it->flags = flags; + + if (it->ss) + it->cset_pos = &css->cgroup->e_csets[css->ss->id]; + else + it->cset_pos = &css->cgroup->cset_links; + + it->cset_head = it->cset_pos; + + css_task_iter_advance_css_set(it); + + while (it->task_pos) { + task = list_entry(it->task_pos, struct task_struct, + cg_list); + + if (likely(!(task->flags & PF_EXITING))) + break; + + css_task_iter_advance(it); + } + + spin_unlock_irq(&css_set_lock); +} + /** * css_task_iter_next - return the next task for the iterator * @it: the task iterator being iterated Thanks -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project