Messages in this thread | | | Date | Thu, 15 May 2008 22:31:36 +0530 | From | Balbir Singh <> | Subject | Re: [-mm][PATCH 4/4] Add memrlimit controller accounting and control (v4) |
| |
Paul Menage wrote: > 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. >
OK, that's definitely more meaningful. Thanks for clarifying.
>> > - 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. >
Let me get that done and I'll post the next version.
> Thanks, > > Paul
-- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
| |