Re: [PATCH] mm,oom: use per signal_struct flag rather than clear TIF_MEMDIE

[Michal Hocko <mhocko@kernel.org>]

On Mon 27-06-16 17:51:20, Oleg Nesterov wrote:
> On 06/27, Michal Hocko wrote:
> >
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -237,6 +237,8 @@ void free_task(struct task_struct *tsk)
> >  	ftrace_graph_exit_task(tsk);
> >  	put_seccomp_filter(tsk);
> >  	arch_release_task_struct(tsk);
> > +	if (tsk->active_mm)
> > +		mmdrop(tsk->active_mm);
> >  	free_task_struct(tsk);
> >  }
> >  EXPORT_SYMBOL(free_task);
> > @@ -1022,6 +1024,8 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
> >  good_mm:
> >  	tsk->mm = mm;
> >  	tsk->active_mm = mm;
> > +	/* to be release in the final task_put */
> > +	atomic_inc(&mm->mm_count);
> >  	return 0;
> 
> No, I don't think this can work.
> 
> Note that tsk->active_mm in free_task() points to the random mm "borrowed"
> from the previous/random task in context_switch() if task->mm == NULL. This
> is true for kthreads and for the task which has already called exit_mm().

OK, I misread the code. I though we wouldn't passed that route
again. Anyway, back to the drawing board.

> 
> > -	p = find_lock_task_mm(tsk);
> > -	if (!p)
> > -		goto unlock_oom;
> > -	mm = p->mm;
> > +	task_lock(tsk);
> > +	mm = tsk->active_mm;
> 
> The same. We can't know where this ->active_mm points to.
> 
> Just suppose that this tsk schedules after exit_mm(). When it gets CPU
> again tsk->active_mm will point to ->mm of another task which in turns
> called schedule() to make this tsk active.
> 
> Yes I agree, it would be nice to remove find_lock_task_mm(). And in
> fact it would be nice to kill task_struct->mm (but this needs a lot
> of cleanups). We probably want signal_struct->mm, but this is a bit
> complicated (locking).

Is there any hard requirement to reset task_struct::mm in the first
place?

I mean I could have added oom_mm pointer into the task_struct and that
would guarantee that we always have a valid pointer when it is needed
but having yet another mm pointer there.
-- 
Michal Hocko
SUSE Labs

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