Re: [RFC][PATCH] cgroup_threadgroup_rwsem - affects scalability and OOM

On Tue, Aug 09, 2016 at 05:00:59PM +0800, Zefan Li wrote:

quoted

This almost stalls the system, this patch moves the threadgroup_change_begin
from before cgroup_fork() to just before cgroup_canfork(). Ideally we shouldn't
have to worry about threadgroup changes till the task is actually added to
the threadgroup. This avoids having to call reclaim with cgroup_threadgroup_rwsem
held.

There are other theoretical issues with this semaphore

systemd can do

1. cgroup_mutex (cgroup_kn_lock_live)
2. cgroup_threadgroup_rwsem (W) (__cgroup_procs_write)

and other threads can go

1. cgroup_threadgroup_rwsem (R) (copy_process)
2. mem_cgroup_iter (as a part of reclaim) (cgroup_mutex -- rcu lock or cgroup_mutex)

However, I've not examined them in too much detail or looked at lockdep
wait chains for those paths.

I am sure there is a good reason for placing cgroup_threadgroup_rwsem
where it is today and I might be missing something. I am also surprised
no-one else has run into it so far.

Comments?

We used to use cgroup_threadgroup_rwsem for syncronization between threads
in the same threadgroup, but now it has evolved to ensure atomic operations
across multi processes.

Yes and it seems incorrect

For example, I'm trying to fix a race. See https://lkml.org/lkml/2016/8/8/900

And the fix kind of relies on the fact that cgroup_post_fork() is placed
inside the read section of cgroup_threadgroup_rwsem, so that cpuset_fork()
won't race with cgroup migration.

My patch retains that behaviour, before ss->fork() is called we hold
the cgroup_threadgroup_rwsem, in fact it is held prior to ss->can_fork()


Balbir 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>