[PATCH 5/5] cgroup: separate out cgroup_attach_proc error handling code

Tejun Heo tj at kernel.org
Tue Dec 20 23:35:02 UTC 2011


Hello,

On Tue, Dec 20, 2011 at 03:14:33PM -0800, Mandeep Singh Baines wrote:
> @@ -2067,9 +2067,10 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
>  	read_unlock(&tasklist_lock);
>  
>  	/* methods shouldn't be called if no task is actually migrating */
> -	retval = 0;
> -	if (!group_size)
> +	if (!group_size) {
> +		retval = 0;
>  		goto out_free_group_list;
> +	}

Eh... I don't think this is an improvement.  It's just different.

> @@ -2126,20 +2127,20 @@ int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader)
>  	 */
>  	synchronize_rcu();
>  	cgroup_wakeup_rmdir_waiter(cgrp);
> -	retval = 0;
> +	flex_array_free(group);
> +	return 0;

Hmm... maybe goto out_free_group_list?  Duplicating cleanup on success
and failure paths can lead future updaters forget one of them.  The
exit path in this function isn't pretty but I don't think the proposed
patch improves it either.

Thanks.

-- 
tejun


More information about the Containers mailing list