Date: Fri, 19 Apr 2013 13:58:21 -0700
From: Tejun Heo <tj@kernel.org>
To: Li Zefan <lizefan@huawei.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Cgroups <cgroups@vger.kernel.org>,
        Containers <containers@lists.linux-foundation.org>,
        Glauber Costa <glommer@parallels.com>
Subject: Re: [PATCH 09/10] cpuset: allow to keep tasks in empty cpusets
Message-ID: <20130419205821.GA15543@mtj.dyndns.org>
References: <517137A7.4020605@huawei.com>
 <517138A4.3070209@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <517138A4.3070209@huawei.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1402
Lines: 42

Hello,

On Fri, Apr 19, 2013 at 08:29:24PM +0800, Li Zefan wrote:
> +static void update_tasks_cpumask_hier(struct cpuset *root_cs,
> +				      bool update_root, struct ptr_heap *heap)
> +{
> +	struct cpuset *cp;
> +	struct cgroup *pos_cgrp;
> +
> +	if (update_root)
> +		update_tasks_cpumask(root_cs, heap);
> +
> +	rcu_read_lock();
> +	cpuset_for_each_descendant_pre(cp, pos_cgrp, root_cs) {
> +		/* skip the whole subtree if @cp have some CPU */
> +		if (!cpumask_empty(cp->cpus_allowed)) {
> +			pos_cgrp = cgroup_rightmost_descendant(pos_cgrp);
> +			continue;
> +		}
> +
> +		update_tasks_cpumask(cp, heap);
> +	}
> +	rcu_read_unlock();

I don't think we can call update_tasks_cpumask() under
rcu_read_lock().  It calls into set_cpus_allowed_ptr() which may
block, so you'll probably have to punt it to a work item like how
migration is being done.  Another approach would be converting cgroup
to use SRCU instead, which would lessen pain on other places too.  The
only problem there would be that srcu_read_lock() is a bit more
expensive than rcu_read_lock().  I'm not sure whether that'd show up
in some hot path or not.  Ideas?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/