From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frederic Weisbecker Subject: Re: [PATCH v2] cgroup: remove redundate get/put of old css_set from migrate Date: Tue, 20 Dec 2011 19:45:00 +0100 Message-ID: <20111220184457.GA17668@somewhere> References: <20111219172027.GJ24519@google.com> <1324320274-18485-1-git-send-email-msb@chromium.org> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/0snHqNATgdnhsBQIyDLk8bXlv/rpa44vZBThJRAKqs=; b=S5SzKBWgiZzkqdADwXxen1LMRxeNF1lPTJExWLxbe7+HuWNTIOJbgS9vqHvHY5mcKD 06MzkAgPY3SY4Qeudau6MIyQDEI0rvydQkPOmuw1d19Xi5r5OA58otu0fU+26axj+nKW T+uH/nnC/CyzcFTewkEohkV6K+03sI/Aasm0o= Content-Disposition: inline In-Reply-To: <1324320274-18485-1-git-send-email-msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Mandeep Singh Baines Cc: Tejun Heo , Li Zefan , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, KAMEZAWA Hiroyuki , Oleg Nesterov , Andrew Morton , Paul Menage On Mon, Dec 19, 2011 at 10:44:34AM -0800, Mandeep Singh Baines wrote: > Changes in V2: > * Updated commit message as per Tejun's feedback: > * https://lkml.org/lkml/2011/12/19/289 > > -- >8 -- (snip) > > We can now assume that the css_set reference held by the task > will not go away for an exiting task. PF_EXITING state can be > trusted throughout migration by checking it after locking > threadgroup. > > While at it, renamed css_set_check_fetched to css_set_fetched. > !css_set_fetched() seems to read better than > !css_set_check_fetched(). > > This patch depends on: > > commit cd3d095275374220921fcf0d4e0c16584b26ddbc > Author: Tejun Heo > Date: Mon Dec 12 18:12:21 2011 -0800 > > cgroup: always lock threadgroup during migration > > Signed-off-by: Mandeep Singh Baines > Cc: Tejun Heo > Cc: Li Zefan > Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org > Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: KAMEZAWA Hiroyuki > Cc: Frederic Weisbecker > Cc: Oleg Nesterov > Cc: Andrew Morton > Cc: Paul Menage > --- > kernel/cgroup.c | 25 ++++++------------------- > 1 files changed, 6 insertions(+), 19 deletions(-) > > diff --git a/kernel/cgroup.c b/kernel/cgroup.c > index 1b3b841..eb95e32 100644 > --- a/kernel/cgroup.c > +++ b/kernel/cgroup.c > @@ -1856,7 +1856,6 @@ static int cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp, > */ > task_lock(tsk); > oldcg = tsk->cgroups; > - get_css_set(oldcg); > task_unlock(tsk); > > /* locate or allocate a new css_set for this task. */ > @@ -1872,12 +1871,9 @@ static int cgroup_task_migrate(struct cgroup *cgrp, struct cgroup *oldcgrp, > might_sleep(); > /* find_css_set will give us newcg already referenced. */ > newcg = find_css_set(oldcg, cgrp); > - if (!newcg) { > - put_css_set(oldcg); > + if (!newcg) > return -ENOMEM; > - } > } > - put_css_set(oldcg); > > /* @tsk can't exit as its threadgroup is locked */ > task_lock(tsk); > @@ -2015,9 +2011,8 @@ struct cg_list_entry { > struct list_head links; > }; > > -static bool css_set_check_fetched(struct cgroup *cgrp, > - struct task_struct *tsk, struct css_set *cg, > - struct list_head *newcg_list) > +static bool css_set_fetched(struct cgroup *cgrp, struct task_struct *tsk, > + struct css_set *cg, struct list_head *newcg_list) > { > struct css_set *newcg; > struct cg_list_entry *cg_entry; > @@ -2191,19 +2186,11 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader) > /* get old css_set pointer */ > task_lock(tc->task); > oldcg = tc->task->cgroups; > - get_css_set(oldcg); > task_unlock(tc->task); > - /* see if the new one for us is already in the list? */ > - if (css_set_check_fetched(cgrp, tc->task, oldcg, &newcg_list)) { > - /* was already there, nothing to do. */ > - put_css_set(oldcg); > - } else { > - /* we don't already have it. get new one. */ > - retval = css_set_prefetch(cgrp, oldcg, &newcg_list); > - put_css_set(oldcg); > - if (retval) > + /* if we don't already have it in the list get a new one */ > + if (!css_set_fetched(cgrp, tc->task, oldcg, &newcg_list)) > + if (css_set_prefetch(cgrp, oldcg, &newcg_list)) Forgot to put the error value in retval? > goto out_list_teardown; > - } > } > > /* > -- > 1.7.3.1 > Thanks.