Re: [rfc][patch 3/3] mm, memcg: introduce own oom handler to iterate only over its own threads

[Oleg Nesterov <oleg@redhat.com>]

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@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>