[Devel] [PATCH rh7 1/3] ve: Attach a ve cgroup from task_work

[Vladimir Davydov]

On Fri, Oct 16, 2015 at 07:24:49PM +0300, Kirill Tkhai wrote:
...
> @@ -733,6 +742,13 @@ static int ve_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
>  		}
>  	}
>  
> +	cgroup_taskset_for_each(task, cg, tset) {
> +		/* Attaching is in process */
> +		if (task->task_ve_work.func != NULL)
> +			return -EAGAIN;

Failing write to cgroup.procs with EAGAIN looks unexpected. For sure,
nobody will retry.

> +	}
> +	smp_mb(); /* Pairs with smp_mb() in ve_attach_work() */
> +
>  	return 0;
>  }
>  
> @@ -753,7 +769,8 @@ static void ve_attach(struct cgroup *cg, struct cgroup_taskset *tset)
>  		/* Leave parent exec domain */
>  		task->parent_exec_id--;
>  
> -		task->task_ve = ve;
> +		init_task_work(&task->task_ve_work, ve_attach_work);

Why do you need to keep the work on task_struct? It's already huge, so
let's please try not to make it even bigger. I don't see any reason why
kmalloc-ing a task_work wouldn't suffice in this case. Moreover, AFAICS
it would liberate us from the necessity to use memory barriers and allow
us to avoid EAGAIN failures.

Hell, I placed ub's cgroup_attach_work in task_struct either, what a
mess...

Besides, I don't see you handle a race with fork (see ub_cgroup_fork).
Why?

> +		task_work_add(task, &task->task_ve_work, true);
>  	}
>  }
>  
>