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
