On 06/27, David Rientjes wrote:
>
> On Tue, 26 Jun 2012, David Rientjes wrote:
>
> > 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).
> @@ -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,
> @@ -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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>