| Message ID | 20230814063428.4111206-2-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: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
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
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);