Re: [rfc][patch 3/3] mm, memcg: introduce own oom handler to iterate only over its own threads
From: Oleg Nesterov <oleg@redhat.com>
Date: 2012-06-28 17:18:43
Also in:
linux-mm
On 06/27, David Rientjes wrote:
On Tue, 26 Jun 2012, David Rientjes wrote:quoted
It turns out that task->children is not an rcu-protected list so this doesn't work.
Yes. And just in case, we can't rcuify ->children because of re-parenting.
It's a tough patch to review, but the basics are that - oom_kill_process() is made to no longer need tasklist_lock; it's only taken for the iteration over children and everything else, including dump_header() is protected by rcu_read_lock() for kernels enabling /proc/sys/vm/oom_dump_tasks, - oom_kill_process() assumes that we have a reference to p, the victim, when it's called. It can release this reference and grab a child's reference if necessary and drops it before returning, and - select_bad_process() does not require tasklist_lock, it gets protected by rcu_read_lock() as well.
Looks correct at first glance... (ignoring the fact we need the fixes in while_each_thread/rcu interaction but this is off-topic and should be fixed anyway).
quoted hunk ↗ jump to hunk
@@ -348,6 +348,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints, struct task_struct *chosen = NULL; unsigned long chosen_points = 0; + rcu_read_lock(); do_each_thread(g, p) { unsigned int points;@@ -370,6 +371,9 @@ static struct task_struct *select_bad_process(unsigned int *ppoints, chosen_points = points; } } while_each_thread(g, p); + if (chosen) + get_task_struct(chosen);
OK, so the caller should do put_task_struct(). But, unless I misread the patch,
quoted hunk ↗ jump to hunk
@@ -454,6 +458,7 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,... + rcu_read_lock(); + p = find_lock_task_mm(victim); + if (!p) { + rcu_read_unlock(); + put_task_struct(victim); return; + } else + victim = p;
And, before return,
+ put_task_struct(victim);
Doesn't look right if victim != p. Oleg. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>