[CRIU] [PATCH 3/3 v2] cg: Add rollback in get_cg_set

[Andrew Vagin]

On Mon, Apr 04, 2016 at 11:33:00AM +0300, Cyrill Gorcunov wrote:
> On Sat, Apr 02, 2016 at 12:17:18AM -0700, Andrew Vagin wrote:
> > >  
> > >  		if (collect && collect_cgroups(&cs->ctls)) {
> > > -			put_ctls(ctls);
> > > +			list_del(&cs->l);
> > > +			n_sets--;
> > 
> > Can we call collect_cgroups() before filling cs?
> 
> We can, but I think it's more natural to splice here:
>  - allocate new cg_set
>  - splice existing data into it
>  - now operate with @cg_set only
> 
> > 
> > > +			put_ctls(&cs->ctls);
> > >  			xfree(cs);
> > > +			INIT_LIST_HEAD(ctls);
> > 
> > I think it's better to use list_splice_init() instead of
> > list_splice()
> 
> yup, thanks. attached.

Reviewed-by: Andrew Vagin <avagin at openvz.org>

Thanks!

> From 34e71b48c4fe1f16cbd7ca34d5f5df335c8b5442 Mon Sep 17 00:00:00 2001
> From: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> Date: Thu, 31 Mar 2016 16:32:41 +0300
> Subject: [PATCH] cg: Add rollback in get_cg_set
> 
> In case of error inside collect_cgroups we need
> 
> - @ctls list has been spliced into @cs->ctls so we cant just free it,
>    but rather use put_ctls on @cs->ctls
> 
> - delete @cs->l from global @cg_sets list before free it
> 
> https://jira.sw.ru/browse/PSBM-45553
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> ---
>  criu/cgroup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/cgroup.c b/criu/cgroup.c
> index ccac9fa..bb76606 100644
> --- a/criu/cgroup.c
> +++ b/criu/cgroup.c
> @@ -201,7 +201,7 @@ static struct cg_set *get_cg_set(struct list_head *ctls, unsigned int n_ctls, bo
>  	if (cs) {
>  		cs->id = cg_set_ids++;
>  		INIT_LIST_HEAD(&cs->ctls);
> -		list_splice(ctls, &cs->ctls);
> +		list_splice_init(ctls, &cs->ctls);
>  		cs->n_ctls = n_ctls;
>  		list_add_tail(&cs->l, &cg_sets);
>  		n_sets++;
> @@ -214,7 +214,9 @@ static struct cg_set *get_cg_set(struct list_head *ctls, unsigned int n_ctls, bo
>  		}
>  
>  		if (collect && collect_cgroups(&cs->ctls)) {
> -			put_ctls(ctls);
> +			list_del(&cs->l);
> +			n_sets--;
> +			put_ctls(&cs->ctls);
>  			xfree(cs);
>  			return NULL;
>  		}
> -- 
> 2.5.5
>