* [PATCH 0/2] mm: oom: terminate the oom_evaluate_task() loop early
@ 2023-08-14 6:34 Peng Zhang
2023-08-14 6:34 ` [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task() Peng Zhang
2023-08-14 6:34 ` [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
0 siblings, 2 replies; 7+ messages in thread
From: Peng Zhang @ 2023-08-14 6:34 UTC (permalink / raw)
To: linux-mm; +Cc: mhocko, shakeelb, akpm, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
This patch series improves the performance of select_bad_process() by
terminating the oom_evaluate_task() loop early (if points == LONG_MAX).
ZhangPeng (2):
mm: oom: remove unnecessary goto in oom_evaluate_task()
mm: oom: terminate the oom_evaluate_task() loop early
mm/oom_kill.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task()
2023-08-14 6:34 [PATCH 0/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
@ 2023-08-14 6:34 ` Peng Zhang
2023-08-16 6:46 ` Michal Hocko
2023-08-14 6:34 ` [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
1 sibling, 1 reply; 7+ messages in thread
From: Peng Zhang @ 2023-08-14 6:34 UTC (permalink / raw)
To: linux-mm; +Cc: mhocko, shakeelb, akpm, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Remove redundant goto statement in oom_evaluate_task() to simplify the
code a bit. No functional modification involved.
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
---
mm/oom_kill.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 44bde56ecd02..10f7826c4035 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -335,14 +335,12 @@ static int oom_evaluate_task(struct task_struct *task, void *arg)
*/
if (oom_task_origin(task)) {
points = LONG_MAX;
- goto select;
+ } else {
+ points = oom_badness(task, oc->totalpages);
+ if (points == LONG_MIN || points < oc->chosen_points)
+ goto next;
}
- points = oom_badness(task, oc->totalpages);
- if (points == LONG_MIN || points < oc->chosen_points)
- goto next;
-
-select:
if (oc->chosen)
put_task_struct(oc->chosen);
get_task_struct(task);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early
2023-08-14 6:34 [PATCH 0/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
2023-08-14 6:34 ` [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task() Peng Zhang
@ 2023-08-14 6:34 ` Peng Zhang
2023-08-16 6:48 ` Michal Hocko
1 sibling, 1 reply; 7+ messages in thread
From: Peng Zhang @ 2023-08-14 6:34 UTC (permalink / raw)
To: linux-mm; +Cc: mhocko, shakeelb, akpm, wangkefeng.wang, sunnanyong, ZhangPeng
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().
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task()
2023-08-14 6:34 ` [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task() Peng Zhang
@ 2023-08-16 6:46 ` Michal Hocko
2023-08-16 8:56 ` zhangpeng (AS)
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2023-08-16 6:46 UTC (permalink / raw)
To: Peng Zhang; +Cc: linux-mm, shakeelb, akpm, wangkefeng.wang, sunnanyong
On Mon 14-08-23 14:34:27, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Remove redundant goto statement in oom_evaluate_task() to simplify the
> code a bit. No functional modification involved.
Quite honestly, I do not see much point in changing the code this way.
We still have other goto labels and also there are other changes
happening where this label could be still benefitial [1]
[1] http://lkml.kernel.org/r/20230810081319.65668-2-zhouchuyi@bytedance.com
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> ---
> mm/oom_kill.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 44bde56ecd02..10f7826c4035 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -335,14 +335,12 @@ static int oom_evaluate_task(struct task_struct *task, void *arg)
> */
> if (oom_task_origin(task)) {
> points = LONG_MAX;
> - goto select;
> + } else {
> + points = oom_badness(task, oc->totalpages);
> + if (points == LONG_MIN || points < oc->chosen_points)
> + goto next;
> }
>
> - points = oom_badness(task, oc->totalpages);
> - if (points == LONG_MIN || points < oc->chosen_points)
> - goto next;
> -
> -select:
> if (oc->chosen)
> put_task_struct(oc->chosen);
> get_task_struct(task);
> --
> 2.25.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early
2023-08-14 6:34 ` [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
@ 2023-08-16 6:48 ` Michal Hocko
2023-08-16 8:57 ` zhangpeng (AS)
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2023-08-16 6:48 UTC (permalink / raw)
To: Peng Zhang; +Cc: linux-mm, shakeelb, akpm, wangkefeng.wang, sunnanyong
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
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task()
2023-08-16 6:46 ` Michal Hocko
@ 2023-08-16 8:56 ` zhangpeng (AS)
0 siblings, 0 replies; 7+ messages in thread
From: zhangpeng (AS) @ 2023-08-16 8:56 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, shakeelb, akpm, wangkefeng.wang, sunnanyong
On 2023/8/16 14:46, Michal Hocko wrote:
> On Mon 14-08-23 14:34:27, Peng Zhang wrote:
>> From: ZhangPeng <zhangpeng362@huawei.com>
>>
>> Remove redundant goto statement in oom_evaluate_task() to simplify the
>> code a bit. No functional modification involved.
> Quite honestly, I do not see much point in changing the code this way.
> We still have other goto labels and also there are other changes
> happening where this label could be still benefitial [1]
>
> [1] http://lkml.kernel.org/r/20230810081319.65668-2-zhouchuyi@bytedance.com
Yes. Sorry I didn't notice this before.
>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>> ---
>> mm/oom_kill.c | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 44bde56ecd02..10f7826c4035 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -335,14 +335,12 @@ static int oom_evaluate_task(struct task_struct *task, void *arg)
>> */
>> if (oom_task_origin(task)) {
>> points = LONG_MAX;
>> - goto select;
>> + } else {
>> + points = oom_badness(task, oc->totalpages);
>> + if (points == LONG_MIN || points < oc->chosen_points)
>> + goto next;
>> }
>>
>> - points = oom_badness(task, oc->totalpages);
>> - if (points == LONG_MIN || points < oc->chosen_points)
>> - goto next;
>> -
>> -select:
>> if (oc->chosen)
>> put_task_struct(oc->chosen);
>> get_task_struct(task);
>> --
>> 2.25.1
--
Best Regards,
Peng
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early
2023-08-16 6:48 ` Michal Hocko
@ 2023-08-16 8:57 ` zhangpeng (AS)
0 siblings, 0 replies; 7+ messages in thread
From: zhangpeng (AS) @ 2023-08-16 8:57 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, shakeelb, akpm, wangkefeng.wang, sunnanyong
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
--
Best Regards,
Peng
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-08-16 8:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-14 6:34 [PATCH 0/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
2023-08-14 6:34 ` [PATCH 1/2] mm: oom: remove unnecessary goto in oom_evaluate_task() Peng Zhang
2023-08-16 6:46 ` Michal Hocko
2023-08-16 8:56 ` zhangpeng (AS)
2023-08-14 6:34 ` [PATCH 2/2] mm: oom: terminate the oom_evaluate_task() loop early Peng Zhang
2023-08-16 6:48 ` Michal Hocko
2023-08-16 8:57 ` zhangpeng (AS)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.