Re: [PATCH] mm/oom_kill: break evaluation when a task has been selected

[Zackary Liu <zackary.liu.pro@gmail.com>]

On Jun 1 2022, at 3:45 pm, Michal Hocko <mhocko@suse.com> wrote:

> On Sat 14-05-22 15:52:28, Zhaoyu Liu wrote:
>> oom points no longer need to be calculated if a task is oom_task_origin(),
>> so return 1 to stop the oom_evaluate_task().
> 
> This doesn't really explain why this is really desired. Is this a fix,
> optimization?
> 
> Please also note that this change has some side effects. For one, the
> task marked as oom origin will get killed even if there is still a
> pending oom victim which hasn't been fully dismantled. Is this
> intentional?

Thank you very much for reminding.

From my point of view, the victim was marked in the last oom, and now it
has entered the oom again, which means that the system still has no
deprecated memory available. In order to ensure that the system can
return to normal as soon as possible, killing the origin task
immediately should be A good choice, and the role of this patch is to
end oom_evaluate_task and return true as soon as the origin task is found.
Maybe this patch is not the optimal solution, it has a trade-off.

It is an honor to discuss with you, thank you very much!

zackary

>> Signed-off-by: Zhaoyu Liu <zackary.liu.pro@gmail.com>
>> ---
>>  mm/oom_kill.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 3996301450e8..b407fba21d19 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -308,7 +308,7 @@ static enum oom_constraint
>> constrained_alloc(struct oom_control *oc)
>>  static int oom_evaluate_task(struct task_struct *task, void *arg)
>>  {
>>  	struct oom_control *oc = arg;
>> -	long points;
>> +	long points = 0;
>>  
>>  	if (oom_unkillable_task(task))
>>  		goto next;
>> @@ -349,7 +349,7 @@ static int oom_evaluate_task(struct task_struct
>> *task, void *arg)
>>  	oc->chosen = task;
>>  	oc->chosen_points = points;
>>  next:
>> -	return 0;
>> +	return points == LONG_MAX;
>>  abort:
>>  	if (oc->chosen)
>>  		put_task_struct(oc->chosen);
>> -- 
>> 2.17.1
> 
> -- 
> Michal Hocko
> SUSE Labs
>