* [PATCH] oom: break after selecting process to kill
@ 2014-09-11 21:33 Niv Yehezkel
2014-09-12 1:22 ` Zhang Zhen
2014-09-12 8:08 ` Michal Hocko
0 siblings, 2 replies; 10+ messages in thread
From: Niv Yehezkel @ 2014-09-11 21:33 UTC (permalink / raw)
To: linux-mm, akpm, rientjes, mhocko, hannes, oleg
There is no need to fallback and continue computing
badness for each running process after we have found a
process currently performing the swapoff syscall. We ought to
immediately select this process for killing.
Signed-off-by: Niv Yehezkel <executerx@gmail.com>
---
mm/oom_kill.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 1e11df8..68ac30e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -305,6 +305,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
struct task_struct *g, *p;
struct task_struct *chosen = NULL;
unsigned long chosen_points = 0;
+ bool process_selected = false;
rcu_read_lock();
for_each_process_thread(g, p) {
@@ -315,7 +316,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
case OOM_SCAN_SELECT:
chosen = p;
chosen_points = ULONG_MAX;
- /* fall through */
+ process_selected = true;
+ break;
case OOM_SCAN_CONTINUE:
continue;
case OOM_SCAN_ABORT:
@@ -324,6 +326,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
case OOM_SCAN_OK:
break;
};
+ if (process_selected)
+ break;
points = oom_badness(p, NULL, nodemask, totalpages);
if (!points || points < chosen_points)
continue;
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] oom: break after selecting process to kill
2014-09-11 21:33 [PATCH] oom: break after selecting process to kill Niv Yehezkel
@ 2014-09-12 1:22 ` Zhang Zhen
2014-09-12 7:39 ` Niv Yehezkel
2014-09-12 8:08 ` Michal Hocko
1 sibling, 1 reply; 10+ messages in thread
From: Zhang Zhen @ 2014-09-12 1:22 UTC (permalink / raw)
To: Niv Yehezkel; +Cc: linux-mm, akpm, rientjes, mhocko, hannes, oleg, wangnan0
On 2014/9/12 5:33, Niv Yehezkel wrote:
> There is no need to fallback and continue computing
> badness for each running process after we have found a
> process currently performing the swapoff syscall. We ought to
> immediately select this process for killing.
>
> Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> ---
> mm/oom_kill.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 1e11df8..68ac30e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -305,6 +305,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> struct task_struct *g, *p;
> struct task_struct *chosen = NULL;
> unsigned long chosen_points = 0;
> + bool process_selected = false;
>
> rcu_read_lock();
> for_each_process_thread(g, p) {
> @@ -315,7 +316,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_SELECT:
> chosen = p;
> chosen_points = ULONG_MAX;
> - /* fall through */
> + process_selected = true;
> + break;
> case OOM_SCAN_CONTINUE:
> continue;
> case OOM_SCAN_ABORT:
> @@ -324,6 +326,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_OK:
> break;
> };
> + if (process_selected)
> + break;
Hi,
The following comment shows that we prefer thread group leaders for display purposes.
If we break here and two threads in a thread group are performing the swapoff syscall, maybe we can not get thread
group leaders.
Thanks!
> points = oom_badness(p, NULL, nodemask, totalpages);
> if (!points || points < chosen_points)
> continue;
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 1:22 ` Zhang Zhen
@ 2014-09-12 7:39 ` Niv Yehezkel
0 siblings, 0 replies; 10+ messages in thread
From: Niv Yehezkel @ 2014-09-12 7:39 UTC (permalink / raw)
To: Zhang Zhen; +Cc: linux-mm, akpm, rientjes, mhocko, hannes, oleg, wangnan0
On Fri, Sep 12, 2014 at 09:22:17AM +0800, Zhang Zhen wrote:
> On 2014/9/12 5:33, Niv Yehezkel wrote:
> > There is no need to fallback and continue computing
> > badness for each running process after we have found a
> > process currently performing the swapoff syscall. We ought to
> > immediately select this process for killing.
> >
> > Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> > ---
> > mm/oom_kill.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 1e11df8..68ac30e 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -305,6 +305,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > struct task_struct *g, *p;
> > struct task_struct *chosen = NULL;
> > unsigned long chosen_points = 0;
> > + bool process_selected = false;
> >
> > rcu_read_lock();
> > for_each_process_thread(g, p) {
> > @@ -315,7 +316,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_SELECT:
> > chosen = p;
> > chosen_points = ULONG_MAX;
> > - /* fall through */
> > + process_selected = true;
> > + break;
> > case OOM_SCAN_CONTINUE:
> > continue;
> > case OOM_SCAN_ABORT:
> > @@ -324,6 +326,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_OK:
> > break;
> > };
> > + if (process_selected)
> > + break;
>
> Hi,
> The following comment shows that we prefer thread group leaders for display purposes.
> If we break here and two threads in a thread group are performing the swapoff syscall, maybe we can not get thread
> group leaders.
>
> Thanks!
>
> > points = oom_badness(p, NULL, nodemask, totalpages);
> > if (!points || points < chosen_points)
> > continue;
> >
>
>
Well, this is not the logic implemented in the loop.
Once a process is selected, it fallbacks and continues the loop.
If two threads are performing the swapoff, the latter will be chosen whatsoever.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] oom: break after selecting process to kill
2014-09-11 21:33 [PATCH] oom: break after selecting process to kill Niv Yehezkel
2014-09-12 1:22 ` Zhang Zhen
@ 2014-09-12 8:08 ` Michal Hocko
2014-09-12 8:23 ` Niv Yehezkel
1 sibling, 1 reply; 10+ messages in thread
From: Michal Hocko @ 2014-09-12 8:08 UTC (permalink / raw)
To: Niv Yehezkel; +Cc: linux-mm, akpm, rientjes, hannes, oleg
On Thu 11-09-14 17:33:39, Niv Yehezkel wrote:
> There is no need to fallback and continue computing
> badness for each running process after we have found a
> process currently performing the swapoff syscall. We ought to
> immediately select this process for killing.
a) this is not only about swapoff. KSM (run_store) is currently
considered oom origin as well.
b) you forgot to tell us what led you to this change. It sounds like a
minor optimization to me. We can potentially skip scanning through
many tasks but this is not guaranteed at all because our task might
be at the very end of the tasks list as well.
c) finally this might select thread != thread_group_leader which is a
minor issue affecting oom report
I am not saying the change is wrong but please make sure you first
describe your motivation. Does it fix any issue you are seeing? Is this
just something that struck you while reading the code? Maybe it was
/* always select this thread first */ comment for OOM_SCAN_SELECT.
Besides that your process_selected is not really needed. You could test
for chosen_points == ULONG_MAX as well. This would be even more
straightforward because any score like that is ultimate candidate.
> Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> ---
> mm/oom_kill.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 1e11df8..68ac30e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -305,6 +305,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> struct task_struct *g, *p;
> struct task_struct *chosen = NULL;
> unsigned long chosen_points = 0;
> + bool process_selected = false;
>
> rcu_read_lock();
> for_each_process_thread(g, p) {
> @@ -315,7 +316,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_SELECT:
> chosen = p;
> chosen_points = ULONG_MAX;
> - /* fall through */
> + process_selected = true;
> + break;
> case OOM_SCAN_CONTINUE:
> continue;
> case OOM_SCAN_ABORT:
> @@ -324,6 +326,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_OK:
> break;
> };
> + if (process_selected)
> + break;
> points = oom_badness(p, NULL, nodemask, totalpages);
> if (!points || points < chosen_points)
> continue;
> --
> 1.7.10.4
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 8:08 ` Michal Hocko
@ 2014-09-12 8:23 ` Niv Yehezkel
2014-09-12 12:18 ` Michal Hocko
0 siblings, 1 reply; 10+ messages in thread
From: Niv Yehezkel @ 2014-09-12 8:23 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, akpm, rientjes, hannes, oleg
[-- Attachment #1: Type: text/plain, Size: 3012 bytes --]
On Fri, Sep 12, 2014 at 10:08:53AM +0200, Michal Hocko wrote:
> On Thu 11-09-14 17:33:39, Niv Yehezkel wrote:
> > There is no need to fallback and continue computing
> > badness for each running process after we have found a
> > process currently performing the swapoff syscall. We ought to
> > immediately select this process for killing.
>
> a) this is not only about swapoff. KSM (run_store) is currently
> considered oom origin as well.
> b) you forgot to tell us what led you to this change. It sounds like a
> minor optimization to me. We can potentially skip scanning through
> many tasks but this is not guaranteed at all because our task might
> be at the very end of the tasks list as well.
> c) finally this might select thread != thread_group_leader which is a
> minor issue affecting oom report
>
> I am not saying the change is wrong but please make sure you first
> describe your motivation. Does it fix any issue you are seeing? Is this
> just something that struck you while reading the code? Maybe it was
> /* always select this thread first */ comment for OOM_SCAN_SELECT.
> Besides that your process_selected is not really needed. You could test
> for chosen_points == ULONG_MAX as well. This would be even more
> straightforward because any score like that is ultimate candidate.
>
> > Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> > ---
> > mm/oom_kill.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 1e11df8..68ac30e 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -305,6 +305,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > struct task_struct *g, *p;
> > struct task_struct *chosen = NULL;
> > unsigned long chosen_points = 0;
> > + bool process_selected = false;
> >
> > rcu_read_lock();
> > for_each_process_thread(g, p) {
> > @@ -315,7 +316,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_SELECT:
> > chosen = p;
> > chosen_points = ULONG_MAX;
> > - /* fall through */
> > + process_selected = true;
> > + break;
> > case OOM_SCAN_CONTINUE:
> > continue;
> > case OOM_SCAN_ABORT:
> > @@ -324,6 +326,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_OK:
> > break;
> > };
> > + if (process_selected)
> > + break;
> > points = oom_badness(p, NULL, nodemask, totalpages);
> > if (!points || points < chosen_points)
> > continue;
> > --
> > 1.7.10.4
> >
>
> --
> Michal Hocko
> SUSE Labs
Been reviewing kernel code lately and looking for implementations not fulfilling their actual intention. That's about most of the patches I tend to send.
Motivation is pretty much derived from the Eudyptula challenge so there is not concrete reason for this patch.
To the point: I have not witnessed any major affects to performance due to this.
I fixed the patch and attached it to this mail.
[-- Attachment #2: 0001-break-after-selecting-process-to-kill.patch --]
[-- Type: text/x-diff, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 8:23 ` Niv Yehezkel
@ 2014-09-12 12:18 ` Michal Hocko
2014-09-12 12:21 ` Niv Yehezkel
0 siblings, 1 reply; 10+ messages in thread
From: Michal Hocko @ 2014-09-12 12:18 UTC (permalink / raw)
To: Niv Yehezkel; +Cc: linux-mm, akpm, rientjes, hannes, oleg
On Fri 12-09-14 04:23:29, Niv Yehezkel wrote:
[...]
> From 1e92f232e9367565d93629b54117b27b9bbfebda Mon Sep 17 00:00:00 2001
> From: Niv Yehezkel <executerx@gmail.com>
> Date: Fri, 12 Sep 2014 04:21:48 -0400
> Subject: [PATCH] break after selecting process to kill
>
>
Now the justification please ;)
> Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> ---
> mm/oom_kill.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 1e11df8..3203578 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -315,7 +315,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_SELECT:
> chosen = p;
> chosen_points = ULONG_MAX;
> - /* fall through */
> + break;
> case OOM_SCAN_CONTINUE:
> continue;
> case OOM_SCAN_ABORT:
> @@ -324,6 +324,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> case OOM_SCAN_OK:
> break;
> };
> + if (chosen_points == ULONG_MAX)
> + break;
> points = oom_badness(p, NULL, nodemask, totalpages);
> if (!points || points < chosen_points)
> continue;
> --
> 1.7.10.4
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 12:18 ` Michal Hocko
@ 2014-09-12 12:21 ` Niv Yehezkel
2014-09-12 12:31 ` Michal Hocko
2014-09-14 20:46 ` David Rientjes
0 siblings, 2 replies; 10+ messages in thread
From: Niv Yehezkel @ 2014-09-12 12:21 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-mm, akpm, rientjes, hannes, oleg
On Fri, Sep 12, 2014 at 02:18:17PM +0200, Michal Hocko wrote:
> On Fri 12-09-14 04:23:29, Niv Yehezkel wrote:
> [...]
> > From 1e92f232e9367565d93629b54117b27b9bbfebda Mon Sep 17 00:00:00 2001
> > From: Niv Yehezkel <executerx@gmail.com>
> > Date: Fri, 12 Sep 2014 04:21:48 -0400
> > Subject: [PATCH] break after selecting process to kill
> >
> >
>
> Now the justification please ;)
>
> > Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> > ---
> > mm/oom_kill.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 1e11df8..3203578 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -315,7 +315,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_SELECT:
> > chosen = p;
> > chosen_points = ULONG_MAX;
> > - /* fall through */
> > + break;
> > case OOM_SCAN_CONTINUE:
> > continue;
> > case OOM_SCAN_ABORT:
> > @@ -324,6 +324,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > case OOM_SCAN_OK:
> > break;
> > };
> > + if (chosen_points == ULONG_MAX)
> > + break;
> > points = oom_badness(p, NULL, nodemask, totalpages);
> > if (!points || points < chosen_points)
> > continue;
> > --
> > 1.7.10.4
> >
>
>
> --
> Michal Hocko
> SUSE Labs
As mentioned earlier, there's no need to keep iterating over all
running processes once the process with the highest score has been found.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 12:21 ` Niv Yehezkel
@ 2014-09-12 12:31 ` Michal Hocko
2014-09-14 20:46 ` David Rientjes
1 sibling, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2014-09-12 12:31 UTC (permalink / raw)
To: Niv Yehezkel; +Cc: linux-mm, akpm, rientjes, hannes, oleg
On Fri 12-09-14 08:21:43, Niv Yehezkel wrote:
> On Fri, Sep 12, 2014 at 02:18:17PM +0200, Michal Hocko wrote:
> > On Fri 12-09-14 04:23:29, Niv Yehezkel wrote:
> > [...]
> > > From 1e92f232e9367565d93629b54117b27b9bbfebda Mon Sep 17 00:00:00 2001
> > > From: Niv Yehezkel <executerx@gmail.com>
> > > Date: Fri, 12 Sep 2014 04:21:48 -0400
> > > Subject: [PATCH] break after selecting process to kill
> > >
> > >
> >
> > Now the justification please ;)
> >
> > > Signed-off-by: Niv Yehezkel <executerx@gmail.com>
> > > ---
> > > mm/oom_kill.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > index 1e11df8..3203578 100644
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -315,7 +315,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > case OOM_SCAN_SELECT:
> > > chosen = p;
> > > chosen_points = ULONG_MAX;
> > > - /* fall through */
> > > + break;
> > > case OOM_SCAN_CONTINUE:
> > > continue;
> > > case OOM_SCAN_ABORT:
> > > @@ -324,6 +324,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > case OOM_SCAN_OK:
> > > break;
> > > };
> > > + if (chosen_points == ULONG_MAX)
> > > + break;
> > > points = oom_badness(p, NULL, nodemask, totalpages);
> > > if (!points || points < chosen_points)
> > > continue;
> > > --
> > > 1.7.10.4
> > >
> >
> >
> > --
> > Michal Hocko
> > SUSE Labs
>
> As mentioned earlier, there's no need to keep iterating over all
> running processes once the process with the highest score has been found.
Please refer to Documentation/SubmittingPatches, especially "2) Describe
your changes." section for more information about the preferred
workflow. I really do not want to be nit picking on you but this is not
the right way to send your changes.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] oom: break after selecting process to kill
2014-09-12 12:21 ` Niv Yehezkel
2014-09-12 12:31 ` Michal Hocko
@ 2014-09-14 20:46 ` David Rientjes
2014-09-23 18:30 ` Michal Hocko
1 sibling, 1 reply; 10+ messages in thread
From: David Rientjes @ 2014-09-14 20:46 UTC (permalink / raw)
To: Niv Yehezkel; +Cc: Michal Hocko, linux-mm, akpm, hannes, oleg
On Fri, 12 Sep 2014, Niv Yehezkel wrote:
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > index 1e11df8..3203578 100644
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -315,7 +315,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > case OOM_SCAN_SELECT:
> > > chosen = p;
> > > chosen_points = ULONG_MAX;
> > > - /* fall through */
> > > + break;
> > > case OOM_SCAN_CONTINUE:
> > > continue;
> > > case OOM_SCAN_ABORT:
> > > @@ -324,6 +324,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > case OOM_SCAN_OK:
> > > break;
> > > };
> > > + if (chosen_points == ULONG_MAX)
> > > + break;
> > > points = oom_badness(p, NULL, nodemask, totalpages);
> > > if (!points || points < chosen_points)
> > > continue;
> > > --
> > > 1.7.10.4
> > >
> >
> >
> > --
> > Michal Hocko
> > SUSE Labs
>
> As mentioned earlier, there's no need to keep iterating over all
> running processes once the process with the highest score has been found.
>
This would lead to unnecessary oom killing since we may miss a process
that returns OOM_SCAN_ABORT simply because it is later in the tasklist (we
want to defer oom killing if there is an exiting process or an oom kill
victim hasn't exited yet). NACK.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] oom: break after selecting process to kill
2014-09-14 20:46 ` David Rientjes
@ 2014-09-23 18:30 ` Michal Hocko
0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2014-09-23 18:30 UTC (permalink / raw)
To: David Rientjes; +Cc: Niv Yehezkel, linux-mm, akpm, hannes, oleg
On Sun 14-09-14 13:46:15, David Rientjes wrote:
> On Fri, 12 Sep 2014, Niv Yehezkel wrote:
>
> > > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > > index 1e11df8..3203578 100644
> > > > --- a/mm/oom_kill.c
> > > > +++ b/mm/oom_kill.c
> > > > @@ -315,7 +315,7 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > > case OOM_SCAN_SELECT:
> > > > chosen = p;
> > > > chosen_points = ULONG_MAX;
> > > > - /* fall through */
> > > > + break;
> > > > case OOM_SCAN_CONTINUE:
> > > > continue;
> > > > case OOM_SCAN_ABORT:
> > > > @@ -324,6 +324,8 @@ static struct task_struct *select_bad_process(unsigned int *ppoints,
> > > > case OOM_SCAN_OK:
> > > > break;
> > > > };
> > > > + if (chosen_points == ULONG_MAX)
> > > > + break;
> > > > points = oom_badness(p, NULL, nodemask, totalpages);
> > > > if (!points || points < chosen_points)
> > > > continue;
> > > > --
> > > > 1.7.10.4
> > > >
> > >
> > >
> > > --
> > > Michal Hocko
> > > SUSE Labs
> >
> > As mentioned earlier, there's no need to keep iterating over all
> > running processes once the process with the highest score has been found.
> >
>
> This would lead to unnecessary oom killing since we may miss a process
> that returns OOM_SCAN_ABORT simply because it is later in the tasklist (we
> want to defer oom killing if there is an exiting process or an oom kill
> victim hasn't exited yet). NACK.
Good point David. I have completely missed this part!
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-23 18:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-11 21:33 [PATCH] oom: break after selecting process to kill Niv Yehezkel
2014-09-12 1:22 ` Zhang Zhen
2014-09-12 7:39 ` Niv Yehezkel
2014-09-12 8:08 ` Michal Hocko
2014-09-12 8:23 ` Niv Yehezkel
2014-09-12 12:18 ` Michal Hocko
2014-09-12 12:21 ` Niv Yehezkel
2014-09-12 12:31 ` Michal Hocko
2014-09-14 20:46 ` David Rientjes
2014-09-23 18:30 ` Michal Hocko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).