Re: [PATCH v2] mm,oom: exclude oom_task_origin processes if they are OOM-unkillable.

[Michal Hocko]

On Mon 22-02-16 17:06:29, David Rientjes wrote:
> On Thu, 18 Feb 2016, Michal Hocko wrote:
> 
> > > Anyway, this is NACK'd since task->signal->oom_score_adj is checked under 
> > > task_lock() for threads with memory attached, that's the purpose of 
> > > finding the correct thread in oom_badness() and taking task_lock().  We 
> > > aren't going to duplicate logic in several functions that all do the same 
> > > thing.
> > 
> > Is the task_lock really necessary, though? E.g. oom_task_origin()
> > doesn't seem to depend on it for task->signal safety. If you are
> > referring to races with changing oom_score_adj does such a race matter
> > at all?
> > 
> 
> oom_badness() ranges from 0 (don't kill) to 1000 (please kill).  It 
> factors in the setting of /proc/self/oom_score_adj to change that value.  
> That is where OOM_SCORE_ADJ_MIN is enforced. 

The question is whether the current placement of OOM_SCORE_ADJ_MIN
is appropriate. Wouldn't it make more sense to check it in oom_unkillable_task
instead? Sure, checking oom_score_adj under task_lock inside oom_badness will
prevent from races but the question I raised previously was whether we
actually care about those races? When would it matter? Is it really
likely that the update happen during the oom killing? And if yes what
prevents from the update happening _after_ the check?

If for nothing else oom_unkillable_task would be complete that way. E.g.
sysctl_oom_kill_allocating_task has to check for OOM_SCORE_ADJ_MIN
because it doesn't rely on oom_badness and that alone would suggest
that the check is misplaced.

That being said I do not really care that much. I would just find it
neater to have oom_unkillable_task that would really consider all the
cases where the OOM should ignore a task.
-- 
Michal Hocko
SUSE Labs