oss-sec mailing list archives
Re: [PATCH 4/4] oom: don't ignore rss in nascent mm
From: Oleg Nesterov <oleg () redhat com>
Date: Thu, 16 Sep 2010 19:44:33 +0200
On 09/16, KOSAKI Motohiro wrote:
ChangeLog o since v1 - Always use thread group leader's ->in_exec_mm.
Confused ;)
+static unsigned long oom_rss_swap_usage(struct task_struct *p) +{ + struct task_struct *t = p; + struct task_struct *leader = p->group_leader; + unsigned long points = 0; + + do { + task_lock(t); + if (t->mm) { + points += get_mm_rss(t->mm); + points += get_mm_counter(t->mm, MM_SWAPENTS); + task_unlock(t); + break; + } + task_unlock(t); + } while_each_thread(p, t); + + /* + * If the process is in execve() processing, we have to concern + * about both old and new mm. + */ + task_lock(leader); + if (leader->in_exec_mm) { + points += get_mm_rss(leader->in_exec_mm); + points += get_mm_counter(leader->in_exec_mm, MM_SWAPENTS); + } + task_unlock(leader); + + return points; +}
This patch relies on fact that we can't race with de_thread() (and btw the change in de_thread() looks bogus). Then why ->in_exec_mm lives in task_struct ? To me, this looks a bit strange. I think we should either do not use ->group_leader to hold ->in_exec_mm like your previous patch did, or move ->in_exec_mm into signal_struct. The previous 3/4 ensures that only one thread can set ->in_exec_mm. And I don't think oom_rss_swap_usage() should replace find_lock_task_mm() in oom_badness(), I mean something like this: static unsigned long oom_rss_swap_usage(struct mm_struct *mm) { return get_mm_rss(mm) + get_mm_counter(mm, MM_SWAPENTS); } unsigned int oom_badness(struct task_struct *p, ...) { int points = 0; if (unlikely(p->signal->in_exec_mm)) { task_lock(p->group_leader); if (p->signal->in_exec_mm) points = oom_rss_swap_usage(p->signal->in_exec_mm); task_unlock(p->group_leader); } p = find_lock_task_mm(p); if (!p) return points; ... } but this is the matter of taste. What do you think? Oleg.
Current thread:
- [PATCH 0/4] oom fixes for 2.6.36 KOSAKI Motohiro (Sep 15)
- [PATCH 3/4] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro (Sep 15)
- [PATCH 1/4] oom: remove totalpage normalization from oom_badness() KOSAKI Motohiro (Sep 15)
- Re: [PATCH 1/4] oom: remove totalpage normalization from oom_badness() David Rientjes (Sep 15)
- Re: [PATCH 1/4] oom: remove totalpage normalization from oom_badness() KOSAKI Motohiro (Sep 16)
- Re: [PATCH 1/4] oom: remove totalpage normalization from oom_badness() Pekka Enberg (Sep 16)
- Re: [PATCH 1/4] oom: remove totalpage normalization from oom_badness() David Rientjes (Sep 15)
- [PATCH 2/4] Revert "oom: deprecate oom_adj tunable" KOSAKI Motohiro (Sep 15)
- [PATCH 4/4] oom: don't ignore rss in nascent mm KOSAKI Motohiro (Sep 15)
- Re: [PATCH 4/4] oom: don't ignore rss in nascent mm Oleg Nesterov (Sep 16)
- Re: [PATCH 4/4] oom: don't ignore rss in nascent mm KOSAKI Motohiro (Sep 26)
- Re: [PATCH 4/4] oom: don't ignore rss in nascent mm Oleg Nesterov (Sep 16)