On 2013/1/12 7:48, Colin Cross wrote: > Quick background: 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. > > The call to synchronize_rcu() in cgroup_attach_task and > cgroup_attach_proc can take up to 100ms, and averages 50ms on Nexus 10 > (ARM Exynos5250 SoC) when the cpus are all idle, and similar times > have been measured on multiple other ARM SMP SoCs. > > I worked with Paul around v3.0 to try to remove the synchronize_rcu > calls (see https://android.googlesource.com/kernel/common/+/befae2f2c9137d6af5c8b38670f00441019032bb), > but nobody seemed to have a clear understanding of the full extent of > what the synchronize_rcu was protecting. Since then the relevant > code, especially cgroup_rmdir, has been heavily rewritten, so > hopefully someone understands it better now. > > The only rcu-protected variable updated in cgroup_attach_task is > tsk->cgroups. Freeing the old tsk->cgroups is already rcu-protected > by kfree_rcu in __put_css_set. The only thing that happens before > cgroup_attach_task returns out to userspace is threadgroup_unlock and > cgroup_unlock. Is anything relying on cgroup_mutex to be held for an > RCU grace period? If not, it seems like the synchronize_rcu isn't > protecting anything and is just wasting time. > Let's remove these two synchronize_rcu()s without hesitation. If some subtleties are broken, we'll fix them, and thus make the code saner. Patch will be sent out soon. -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
