lkml.org 
[lkml]   [2013]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] cgroup: remove synchronize_rcu() from cgroup_attach_{task|proc}()
Hey, Li.

On Mon, Jan 14, 2013 at 05:23:26PM +0800, Li Zefan wrote:
> These 2 syncronize_rcu()s make attaching a task to a cgroup
> quite slow, and it can't be ignored in some situations.
>
> A real case from Colin Cross: Android uses cgroups heavily to
> manage thread priorities, putting threads in a background group
> with reduced cpu.shares when they are not visible to the user,
> and in a foreground group when they are. Some RPCs from foreground
> threads to background threads will temporarily move the background
> thread into the foreground group for the duration of the RPC.
> This results in many calls to cgroup_attach_task.
>
> In cgroup_attach_task() it's task->cgroups that is protected by RCU,
> and put_css_set() calls kfree_rcu() to free it.
>
> If we remove this synchronize_rcu(), there can be threads in RCU-read
> sections accessing their old cgroup via current->cgroups with
> concurrent rmdir operation, but this is safe.

Yeah, these synchronize_rcu()s are utterly confused.
synchornize_rcu() necessarily have to come between two operations to
guarantee that the changes made before it are visible to all rcu
readers before proceeding beyond it. Here, synchornize_rcu() are at
the end of attach operations with nothing beyond it. Duh.... its only
effect would be delaying completion of write(2) to sysfs tasks/procs
files until all rcu readers see the change, which is utterly
irrelevant. It doesn't mean anything.

Applying to cgroup/for-3.9.

Thanks.

--
tejun


\
 
 \ /
  Last update: 2013-01-14 20:21    [W:0.073 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site