* [patch -mm 1/2] oom: protect oom_disable_count with task_lock in fork
@ 2010-09-02 0:00 David Rientjes
2010-09-02 0:00 ` [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec David Rientjes
0 siblings, 1 reply; 6+ messages in thread
From: David Rientjes @ 2010-09-02 0:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: KOSAKI Motohiro, KAMEZAWA Hiroyuki, linux-mm
task_lock(p) protects p->mm->oom_disable_count such that it accurately
represents the number of threads attached to that mm that cannot be
killed by the oom killer. p->signal->oom_score_adj is never changed
without holding the lock.
This was missed in the fork() path, so we take the lock to ensure
checking its oom_score_adj and decrementing oom_disable_count don't race.
Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
kernel/fork.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1304,8 +1304,10 @@ bad_fork_cleanup_namespaces:
exit_task_namespaces(p);
bad_fork_cleanup_mm:
if (p->mm) {
+ task_lock(p);
if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
atomic_dec(&p->mm->oom_disable_count);
+ task_unlock(p);
mmput(p->mm);
}
bad_fork_cleanup_signal:
--
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] 6+ messages in thread* [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec
2010-09-02 0:00 [patch -mm 1/2] oom: protect oom_disable_count with task_lock in fork David Rientjes
@ 2010-09-02 0:00 ` David Rientjes
2010-09-02 0:25 ` KOSAKI Motohiro
0 siblings, 1 reply; 6+ messages in thread
From: David Rientjes @ 2010-09-02 0:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: KOSAKI Motohiro, KAMEZAWA Hiroyuki, linux-mm
active_mm in the exec() path can be for an unrelated thread, so the
oom_disable_count logic should use old_mm instead.
Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
fs/exec.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -752,8 +752,8 @@ static int exec_mmap(struct mm_struct *mm)
tsk->mm = mm;
tsk->active_mm = mm;
activate_mm(active_mm, mm);
- if (tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
- atomic_dec(&active_mm->oom_disable_count);
+ if (old_mm && tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
+ atomic_dec(&old_mm->oom_disable_count);
atomic_inc(&tsk->mm->oom_disable_count);
}
task_unlock(tsk);
--
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] 6+ messages in thread* Re: [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec
2010-09-02 0:00 ` [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec David Rientjes
@ 2010-09-02 0:25 ` KOSAKI Motohiro
2010-09-02 0:50 ` David Rientjes
0 siblings, 1 reply; 6+ messages in thread
From: KOSAKI Motohiro @ 2010-09-02 0:25 UTC (permalink / raw)
To: David Rientjes
Cc: kosaki.motohiro, Andrew Morton, KAMEZAWA Hiroyuki, linux-mm
> active_mm in the exec() path can be for an unrelated thread, so the
> oom_disable_count logic should use old_mm instead.
>
> Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
> fs/exec.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -752,8 +752,8 @@ static int exec_mmap(struct mm_struct *mm)
> tsk->mm = mm;
> tsk->active_mm = mm;
> activate_mm(active_mm, mm);
> - if (tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> - atomic_dec(&active_mm->oom_disable_count);
> + if (old_mm && tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> + atomic_dec(&old_mm->oom_disable_count);
> atomic_inc(&tsk->mm->oom_disable_count);
Looks good. However you need to use tsk->signal->oom_adj == OOM_DISABLE because
I removed OOM_SCORE_ADJ_MIN.
> }
> task_unlock(tsk);
>
> --
> 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>
>
--
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] 6+ messages in thread* Re: [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec
2010-09-02 0:25 ` KOSAKI Motohiro
@ 2010-09-02 0:50 ` David Rientjes
2010-09-08 2:44 ` KOSAKI Motohiro
0 siblings, 1 reply; 6+ messages in thread
From: David Rientjes @ 2010-09-02 0:50 UTC (permalink / raw)
To: KOSAKI Motohiro; +Cc: Andrew Morton, KAMEZAWA Hiroyuki, linux-mm
On Thu, 2 Sep 2010, KOSAKI Motohiro wrote:
> > active_mm in the exec() path can be for an unrelated thread, so the
> > oom_disable_count logic should use old_mm instead.
> >
> > Reported-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Signed-off-by: David Rientjes <rientjes@google.com>
> > ---
> > fs/exec.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -752,8 +752,8 @@ static int exec_mmap(struct mm_struct *mm)
> > tsk->mm = mm;
> > tsk->active_mm = mm;
> > activate_mm(active_mm, mm);
> > - if (tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> > - atomic_dec(&active_mm->oom_disable_count);
> > + if (old_mm && tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> > + atomic_dec(&old_mm->oom_disable_count);
> > atomic_inc(&tsk->mm->oom_disable_count);
>
> Looks good. However you need to use tsk->signal->oom_adj == OOM_DISABLE because
> I removed OOM_SCORE_ADJ_MIN.
>
KOSAKI, I'm not going to argue this with you. VM patches, like where you
revert oom_score_adj, go through Andrew. That's not up for debate.
Thanks for the review.
--
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] 6+ messages in thread* Re: [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec
2010-09-02 0:50 ` David Rientjes
@ 2010-09-08 2:44 ` KOSAKI Motohiro
2010-09-08 3:28 ` David Rientjes
0 siblings, 1 reply; 6+ messages in thread
From: KOSAKI Motohiro @ 2010-09-08 2:44 UTC (permalink / raw)
To: David Rientjes
Cc: kosaki.motohiro, Andrew Morton, KAMEZAWA Hiroyuki, linux-mm
> > Looks good. However you need to use tsk->signal->oom_adj == OOM_DISABLE because
> > I removed OOM_SCORE_ADJ_MIN.
> >
>
> KOSAKI, I'm not going to argue this with you. VM patches, like where you
> revert oom_score_adj, go through Andrew. That's not up for debate.
>
> Thanks for the review.
Don't mind. but general warning: If you continue to crappy objection, We
are going to revert full of your userland breakage entirely instead minimum fix.
--
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] 6+ messages in thread
* Re: [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec
2010-09-08 2:44 ` KOSAKI Motohiro
@ 2010-09-08 3:28 ` David Rientjes
0 siblings, 0 replies; 6+ messages in thread
From: David Rientjes @ 2010-09-08 3:28 UTC (permalink / raw)
To: KOSAKI Motohiro; +Cc: Andrew Morton, KAMEZAWA Hiroyuki, linux-mm
On Wed, 8 Sep 2010, KOSAKI Motohiro wrote:
> Don't mind. but general warning: If you continue to crappy objection, We
> are going to revert full of your userland breakage entirely instead minimum fix.
>
I'm not in the business of responding to your threats. Unfortunately for
your argument, you cannot cite a single example of a current user of
/proc/pid/oom_adj that considers either (i) expected memory usage of the
task, or (ii) system RAM capcacity, which would be required for that value
to make any sense given the current implementation.
--
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] 6+ messages in thread
end of thread, other threads:[~2010-09-08 3:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-02 0:00 [patch -mm 1/2] oom: protect oom_disable_count with task_lock in fork David Rientjes
2010-09-02 0:00 ` [patch -mm 2/2] oom: use old_mm for oom_disable_count in exec David Rientjes
2010-09-02 0:25 ` KOSAKI Motohiro
2010-09-02 0:50 ` David Rientjes
2010-09-08 2:44 ` KOSAKI Motohiro
2010-09-08 3:28 ` David Rientjes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox