[Devel] [RFC] Fix get_exec_env() races

[Vladimir Davydov]

On Thu, Oct 15, 2015 at 05:21:04PM +0300, Kirill Tkhai wrote:
> 
> 
> On 15.10.2015 14:15, Pavel Emelyanov wrote:
> > 
> >> @@ -130,6 +131,34 @@ struct ve_struct {
> >>  #endif
> >>  };
> >>  
> >> +static inline struct ve_struct *get_exec_env(void)
> >> +{
> >> +	struct ve_struct *ve;
> >> +
> >> +	if (++current->ve_attach_lock_depth > 1)
> >> +		return current->task_ve;
> >> +
> >> +	rcu_read_lock();
> >> +again:
> >> +	ve = current->task_ve;
> >> +	read_lock(&ve->attach_lock);
> >> +	if (unlikely(current->task_ve != ve)) {
> >> +		read_unlock(&ve->attach_lock);
> >> +		goto again;
> > 
> > Please, no. 3.10 kernel has task_work-s, ask the task you want to
> > attach to ve to execute the work by moving itself into it and keep
> > this small routine small and simple.
> 
> cgroup_attach_task() is called under cgroup_mutex and threadgroup_lock(),
> so we can't wait attaching task till it complete the task work (it may
> execute any code; to be locking cgroup_mutex, for example).

Do we really want to wait until exec_env of the target task has changed?

Anyway, I think we can use task_work even if we need to be synchronous.
You just need two task works - one for the target and one for the
caller. The former will change the target task's exec_env while the
latter will wait for it to finish.

> 
> Should we give a possibility (an interface) for userspace to get it know,
> the task's finished ve changing?
> 
>  
> >> +	}
> >> +	rcu_read_unlock();
> >> +
> >> +	return ve;
> >> +}
> >> +
> > 
>