Messages in this thread | | | Date | Thu, 15 May 2008 08:28:46 -0700 | From | "Paul Menage" <> | Subject | Re: [-mm][PATCH 4/4] Add memrlimit controller accounting and control (v4) |
| |
On Thu, May 15, 2008 at 1:25 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > > > But the only *new* cases of taking the mmap_sem that this would > > introduce would be: > > > > - on a failed vm limit charge > > Why a failed charge? Aren't we talking of moving all charge/uncharge > under mmap_sem? >
Sorry, I worded that wrongly - I meant "cleaning up a successful charge after an expansion fails for other reasons"
I thought that all the charges and most of the uncharges were already under mmap_sem, and it would just be a few of the cleanup paths that needed to take it.
> > > - when a task moves between two cgroups in the memrlimit hierarchy. > > > > Yes, this would nest cgroup_mutex and mmap_sem. Not sure if that would > be a bad side-effect. >
I think it's already nested that way - e.g. the cpusets code can call various migration functions (which take mmap_sem) while holding cgroup_mutex.
> > Refactor the code to try and use mmap_sem and see what I come up > with. Basically use mmap_sem for all charge/uncharge operations as > well use mmap_sem in read_mode in the move_task() and > mm_owner_changed() callbacks. That should take care of the race > conditions discussed, unless I missed something.
Sounds good.
Thanks,
Paul
| |