Re: [-mm][PATCH 4/4] Add memrlimit controller accounting and control (v4)

[Paul Menage]

On Thu, May 15, 2008 at 1:25 AM, Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/