From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: [PATCH v3 7/7] cpuset: fix to migrate mm correctly in a corner case Date: Sun, 9 Jun 2013 08:49:30 -0700 Message-ID: <20130609154930.GD2835@htj.dyndns.org> References: <51B4475A.8030509@huawei.com> <51B4480F.2050400@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=6MgnuayJCo+b70NHwLukyr/b5+QEY6G/C6J/+2tihJU=; b=Agc4vZ4Uks9EuCIIvPfOItcFIaXYgKiRLTSZUk3a1GxZji9fU2We1kCGze9K+wfEwi z0mRDbGtFdbe6uWgmBD1sNGx2rqQPcBd8pHfk8KdpmOE9mQRXgVgZu1aVhv5YHS6b2ot 2PD6i+nhG6ISxCpqxj6N5NDBTihYjydT8nIMSvCPlgQdYDnbQci98CwWJiAkTwwbLnvh khMWN4ilxqabXZs8QZJ3ES5RUVBfKCCX9bAHEgZjHusxSDZ9Qm2pAp0QKUwnsXkv8Y7S P5U1O1KIsGUtkEDZb56h8ZBr0/x/dlnZggU3axcXrWbWBFKkTyfJ2zPzqgfuOYZb986R OFWw== Content-Disposition: inline In-Reply-To: <51B4480F.2050400-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Li Zefan Cc: Cgroups , Containers , LKML On Sun, Jun 09, 2013 at 05:17:03PM +0800, Li Zefan wrote: > Before moving tasks out of empty cpusets, update_tasks_nodemask() > is called, which calls do_migrate_pages(xx, from, to). Then those > tasks are moved to an ancestor, and do_migrate_pages() is called > again. > > The first time: from = node_to_be_offlined, to = empty. > The second time: from = empty, to = ancestor's nodemask. > > so looks like no pages will be migrated. > > Fix this by: > > - Don't call update_tasks_nodemask() on empty cpusets. > - Pass cs->old_mems_allowed to do_migrate_pages(). > > Signed-off-by: Li Zefan > --- > kernel/cpuset.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/kernel/cpuset.c b/kernel/cpuset.c > index d1d6782..3cd68b8 100644 > --- a/kernel/cpuset.c > +++ b/kernel/cpuset.c > @@ -1563,9 +1563,16 @@ static void cpuset_attach(struct cgroup *cgrp, struct cgroup_taskset *tset) > struct cpuset *mems_oldcs = effective_nodemask_cpuset(oldcs); > > mpol_rebind_mm(mm, &cpuset_attach_nodemask_to); > - if (is_memory_migrate(cs)) > - cpuset_migrate_mm(mm, &mems_oldcs->mems_allowed, > + if (is_memory_migrate(cs)) { > + /* > + * old_mems_allowed is the same with mems_allowed here, > + * except if this task is being moved automatically due > + * to hotplug, and in this case mems_allowed is empty > + * and old_mems_allowed is the offlined node. > + */ Wouldn't it be better if the above explain why rather than what? > + cpuset_migrate_mm(mm, &mems_oldcs->old_mems_allowed, > &cpuset_attach_nodemask_to); > + } > mmput(mm); > } > > @@ -2155,7 +2162,7 @@ retry: > * for empty cpuset to take on ancestor's cpumask. > */ > if ((sane && cpumask_empty(cs->cpus_allowed)) || > - !cpumask_empty(&off_cpus)) > + (!cpumask_empty(&off_cpus) && !cpumask_empty(cs->cpus_allowed))) > update_tasks_cpumask(cs, NULL); No biggie but maybe we want briefly explain the conditions being tested? Thanks. -- tejun