| Message ID | 20230814063428.4111206-3-zhangpeng362@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: oom: terminate the oom_evaluate_task() loop early | expand |
On Mon 14-08-23 14:34:28, Peng Zhang wrote: > From: ZhangPeng <zhangpeng362@huawei.com> > > If task is allocating a lot of memory and has been marked to be killed > first, then it gets the highest score (LONG_MAX). Therefore, there is no > need to continue to calculate the points of other tasks. Just terminate > the oom_evaluate_task() loop early, when the task with the highest score > is found. By doing this, we can get some performance gains in > select_bad_process(). The point of evaluating all tasks is that there might be pre-existing oom victim still being torn down. If you cut this short you might kill more tasks than necessary. So I do not think we want this patch. > To implement it, the return value of oom_evaluate_task() is modified. > When the task with the highest score is found (points == LONG_MAX), > oom_evaluate_task() will return 1 and the loop will terminate early. > > Signed-off-by: ZhangPeng <zhangpeng362@huawei.com> > --- > mm/oom_kill.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index 10f7826c4035..02d11ae043aa 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -346,6 +346,8 @@ static int oom_evaluate_task(struct task_struct *task, void *arg) > get_task_struct(task); > oc->chosen = task; > oc->chosen_points = points; > + if (points == LONG_MAX) > + return 1; > next: > return 0; > abort: > -- > 2.25.1
On 2023/8/16 14:48, Michal Hocko wrote: > On Mon 14-08-23 14:34:28, Peng Zhang wrote: >> From: ZhangPeng <zhangpeng362@huawei.com> >> >> If task is allocating a lot of memory and has been marked to be killed >> first, then it gets the highest score (LONG_MAX). Therefore, there is no >> need to continue to calculate the points of other tasks. Just terminate >> the oom_evaluate_task() loop early, when the task with the highest score >> is found. By doing this, we can get some performance gains in >> select_bad_process(). > The point of evaluating all tasks is that there might be pre-existing > oom victim still being torn down. If you cut this short you might kill > more tasks than necessary. Indeed. Thanks for your feedback. > So I do not think we want this patch. >> To implement it, the return value of oom_evaluate_task() is modified. >> When the task with the highest score is found (points == LONG_MAX), >> oom_evaluate_task() will return 1 and the loop will terminate early. >> >> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com> >> --- >> mm/oom_kill.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/mm/oom_kill.c b/mm/oom_kill.c >> index 10f7826c4035..02d11ae043aa 100644 >> --- a/mm/oom_kill.c >> +++ b/mm/oom_kill.c >> @@ -346,6 +346,8 @@ static int oom_evaluate_task(struct task_struct *task, void *arg) >> get_task_struct(task); >> oc->chosen = task; >> oc->chosen_points = points; >> + if (points == LONG_MAX) >> + return 1; >> next: >> return 0; >> abort: >> -- >> 2.25.1
diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 10f7826c4035..02d11ae043aa 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -346,6 +346,8 @@ static int oom_evaluate_task(struct task_struct *task, void *arg) get_task_struct(task); oc->chosen = task; oc->chosen_points = points; + if (points == LONG_MAX) + return 1; next: return 0; abort: