Re: [PATCH -mmotm 2/8] memcg: move memcg_tasklist mutex

KAMEZAWA Hiroyuki
2009-11-06T08:05:02Z

On Fri, 6 Nov 2009 16:49:34 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> If there is no such a lock:
>
> Assume cgroup foo has exceeded its limit and is about to triggering oom.
> 1. Process A, which has been in cgroup baa and uses large memory,
> is just moved to cgroup foo. Process A can be the candidates for being killed.
> 2. Process B, which has been in cgroup foo and uses large memory,
> is just moved from cgroup foo. Process B can be excluded from the candidates for
> being killed.
>
> Hmm, but considering more, those race window exist anyway even if we holds a lock,
> because try_charge decides wether it should trigger oom or not outside of the lock.
>
yes, that's point.

> If this recharge feature is enabled, I think those problems might be avoided by doing like:
>
> __mem_cgroup_try_charge()
> {
> ...
> if (oom) {
> mutex_lock(&memcg_tasklist);
> if (unlikely(mem_cgroup_check_under_limit)) {
> mutex_unlock(&memcg_tasklist);
> continue
> }
> mem_cgroup_out_of_memory();
> mutex_unlock(&memcg_tasklist);
> record_last_oom();
> }
> ...
> }
>
> but it makes codes more complex and the recharge feature isn't necessarily enabled.
>
> Well, I personally think we can remove these locks completely and make codes simpler.
> What do you think ?

I myself vote for removing this lock ;)

Thanks,
-Kame