Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1362563 > unrolled thread

[PATCH 0/9] oom reaper v6

Started byMichal Hocko <mhocko@kernel.org>
First post2016-03-22 12:10 +0100
Last post2016-03-23 13:10 +0100
Articles 8 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/9] oom reaper v6 Michal Hocko <mhocko@kernel.org> - 2016-03-22 12:10 +0100
    [PATCH 3/9] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-03-22 12:10 +0100
    [PATCH 7/9] oom: make oom_reaper_list single linked Michal Hocko <mhocko@kernel.org> - 2016-03-22 12:10 +0100
    [PATCH 6/9] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task Michal Hocko <mhocko@kernel.org> - 2016-03-22 12:10 +0100
    [PATCH 4/9] mm, oom_reaper: report success/failure Michal Hocko <mhocko@kernel.org> - 2016-03-22 12:10 +0100
    Re: [PATCH 0/9] oom reaper v6 David Rientjes <rientjes@google.com> - 2016-03-22 23:10 +0100
      Re: [PATCH 0/9] oom reaper v6 Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-03-23 12:30 +0100
        Re: [PATCH 0/9] oom reaper v6 Michal Hocko <mhocko@kernel.org> - 2016-03-23 13:10 +0100

#1362563 — [PATCH 0/9] oom reaper v6

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-22 12:10 +0100
Subject[PATCH 0/9] oom reaper v6
Message-ID<rfsmd-3ve-5@gated-at.bofh.it>
Hi,
I am reposting the whole patchset on top of the current Linus tree which should
already contain big pile of Andrew's mm patches. This should serve an easier
reviewability and I also hope that this core part of the work can go to 4.6.

The previous version was posted here [1] Hugh and David have suggested to
drop [2] because the munlock path currently depends on the page lock and
it is better if the initial version was conservative and prevent from
any potential lockups even though it is not clear whether they are real
- nobody has seen oom_reaper stuck on the page lock AFAICK. Me or Hugh
will have a look and try to make the munlock path not depend on the page
lock as a follow up work.

Apart from that the feedback revealed one bug for a very unusual
configuration (sysctl_oom_kill_allocating_task) and that has been fixed
by patch 8 and one potential mis interaction with the pm freezer fixed by
patch 7.

I think the current code base is already very useful for many situations.
The rest of the feedback was mostly about potential enhancements of the
current code which I would really prefer to build on top of the current
series. I plan to finish my mmap_sem killable for write in the upcoming
release cycle and hopefully have it merged in the next merge window.
I believe more extensions will follow.

This code has been sitting in the mmotm (thus linux-next) for a while.
Are there any fundamental objections to have this part merged in this
merge window?

Thanks!

[1] http://lkml.kernel.org/r/1454505240-23446-1-git-send-email-mhocko@kernel.org
[2] http://lkml.kernel.org/r/1454505240-23446-3-git-send-email-mhocko@kernel.org

[toc] | [next] | [standalone]


#1362569 — [PATCH 3/9] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-22 12:10 +0100
Subject[PATCH 3/9] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space
Message-ID<rfsmf-3ve-49@gated-at.bofh.it>
In reply to#1362563
From: Michal Hocko <mhocko@suse.com>

When oom_reaper manages to unmap all the eligible vmas there shouldn't
be much of the freable memory held by the oom victim left anymore so it
makes sense to clear the TIF_MEMDIE flag for the victim and allow the
OOM killer to select another task.

The lack of TIF_MEMDIE also means that the victim cannot access memory
reserves anymore but that shouldn't be a problem because it would get
the access again if it needs to allocate and hits the OOM killer again
due to the fatal_signal_pending resp. PF_EXITING check. We can safely
hide the task from the OOM killer because it is clearly not a good
candidate anymore as everyhing reclaimable has been torn down already.

This patch will allow to cap the time an OOM victim can keep TIF_MEMDIE
and thus hold off further global OOM killer actions granted the oom
reaper is able to take mmap_sem for the associated mm struct. This is
not guaranteed now but further steps should make sure that mmap_sem
for write should be blocked killable which will help to reduce such a
lock contention. This is not done by this patch.

Note that exit_oom_victim might be called on a remote task from
__oom_reap_task now so we have to check and clear the flag atomically
otherwise we might race and underflow oom_victims or wake up
waiters too early.

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Suggested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/oom.h |  2 +-
 kernel/exit.c       |  2 +-
 mm/oom_kill.c       | 73 +++++++++++++++++++++++++++++++++++------------------
 3 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
index 03e6257321f0..45993b840ed6 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -91,7 +91,7 @@ extern enum oom_scan_t oom_scan_process_thread(struct oom_control *oc,
 
 extern bool out_of_memory(struct oom_control *oc);
 
-extern void exit_oom_victim(void);
+extern void exit_oom_victim(struct task_struct *tsk);
 
 extern int register_oom_notifier(struct notifier_block *nb);
 extern int unregister_oom_notifier(struct notifier_block *nb);
diff --git a/kernel/exit.c b/kernel/exit.c
index 10e088237fed..ba3bd29d7e1d 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -434,7 +434,7 @@ static void exit_mm(struct task_struct *tsk)
 	mm_update_next_owner(mm);
 	mmput(mm);
 	if (test_thread_flag(TIF_MEMDIE))
-		exit_oom_victim();
+		exit_oom_victim(tsk);
 }
 
 static struct task_struct *find_alive_thread(struct task_struct *p)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index f7ed6ece0719..2830b1c6483e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -416,20 +416,36 @@ bool oom_killer_disabled __read_mostly;
  * victim (if that is possible) to help the OOM killer to move on.
  */
 static struct task_struct *oom_reaper_th;
-static struct mm_struct *mm_to_reap;
+static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
 
-static bool __oom_reap_vmas(struct mm_struct *mm)
+static bool __oom_reap_task(struct task_struct *tsk)
 {
 	struct mmu_gather tlb;
 	struct vm_area_struct *vma;
+	struct mm_struct *mm;
+	struct task_struct *p;
 	struct zap_details details = {.check_swap_entries = true,
 				      .ignore_dirty = true};
 	bool ret = true;
 
-	/* We might have raced with exit path */
-	if (!atomic_inc_not_zero(&mm->mm_users))
+	/*
+	 * Make sure we find the associated mm_struct even when the particular
+	 * thread has already terminated and cleared its mm.
+	 * We might have race with exit path so consider our work done if there
+	 * is no mm.
+	 */
+	p = find_lock_task_mm(tsk);
+	if (!p)
+		return true;
+
+	mm = p->mm;
+	if (!atomic_inc_not_zero(&mm->mm_users)) {
+		task_unlock(p);
 		return true;
+	}
+
+	task_unlock(p);
 
 	if (!down_read_trylock(&mm->mmap_sem)) {
 		ret = false;
@@ -464,60 +480,66 @@ static bool __oom_reap_vmas(struct mm_struct *mm)
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
 	up_read(&mm->mmap_sem);
+
+	/*
+	 * Clear TIF_MEMDIE because the task shouldn't be sitting on a
+	 * reasonably reclaimable memory anymore. OOM killer can continue
+	 * by selecting other victim if unmapping hasn't led to any
+	 * improvements. This also means that selecting this task doesn't
+	 * make any sense.
+	 */
+	tsk->signal->oom_score_adj = OOM_SCORE_ADJ_MIN;
+	exit_oom_victim(tsk);
 out:
 	mmput(mm);
 	return ret;
 }
 
-static void oom_reap_vmas(struct mm_struct *mm)
+static void oom_reap_task(struct task_struct *tsk)
 {
 	int attempts = 0;
 
 	/* Retry the down_read_trylock(mmap_sem) a few times */
-	while (attempts++ < 10 && !__oom_reap_vmas(mm))
+	while (attempts++ < 10 && !__oom_reap_task(tsk))
 		schedule_timeout_idle(HZ/10);
 
 	/* Drop a reference taken by wake_oom_reaper */
-	mmdrop(mm);
+	put_task_struct(tsk);
 }
 
 static int oom_reaper(void *unused)
 {
 	while (true) {
-		struct mm_struct *mm;
+		struct task_struct *tsk;
 
 		wait_event_freezable(oom_reaper_wait,
-				     (mm = READ_ONCE(mm_to_reap)));
-		oom_reap_vmas(mm);
-		WRITE_ONCE(mm_to_reap, NULL);
+				     (tsk = READ_ONCE(task_to_reap)));
+		oom_reap_task(tsk);
+		WRITE_ONCE(task_to_reap, NULL);
 	}
 
 	return 0;
 }
 
-static void wake_oom_reaper(struct mm_struct *mm)
+static void wake_oom_reaper(struct task_struct *tsk)
 {
-	struct mm_struct *old_mm;
+	struct task_struct *old_tsk;
 
 	if (!oom_reaper_th)
 		return;
 
-	/*
-	 * Pin the given mm. Use mm_count instead of mm_users because
-	 * we do not want to delay the address space tear down.
-	 */
-	atomic_inc(&mm->mm_count);
+	get_task_struct(tsk);
 
 	/*
 	 * Make sure that only a single mm is ever queued for the reaper
 	 * because multiple are not necessary and the operation might be
 	 * disruptive so better reduce it to the bare minimum.
 	 */
-	old_mm = cmpxchg(&mm_to_reap, NULL, mm);
-	if (!old_mm)
+	old_tsk = cmpxchg(&task_to_reap, NULL, tsk);
+	if (!old_tsk)
 		wake_up(&oom_reaper_wait);
 	else
-		mmdrop(mm);
+		put_task_struct(tsk);
 }
 
 static int __init oom_init(void)
@@ -532,7 +554,7 @@ static int __init oom_init(void)
 }
 subsys_initcall(oom_init)
 #else
-static void wake_oom_reaper(struct mm_struct *mm)
+static void wake_oom_reaper(struct task_struct *tsk)
 {
 }
 #endif
@@ -563,9 +585,10 @@ void mark_oom_victim(struct task_struct *tsk)
 /**
  * exit_oom_victim - note the exit of an OOM victim
  */
-void exit_oom_victim(void)
+void exit_oom_victim(struct task_struct *tsk)
 {
-	clear_thread_flag(TIF_MEMDIE);
+	if (!test_and_clear_tsk_thread_flag(tsk, TIF_MEMDIE))
+		return;
 
 	if (!atomic_dec_return(&oom_victims))
 		wake_up_all(&oom_victims_wait);
@@ -748,7 +771,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	rcu_read_unlock();
 
 	if (can_oom_reap)
-		wake_oom_reaper(mm);
+		wake_oom_reaper(victim);
 
 	mmdrop(mm);
 	put_task_struct(victim);
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1362575 — [PATCH 7/9] oom: make oom_reaper_list single linked

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-22 12:10 +0100
Subject[PATCH 7/9] oom: make oom_reaper_list single linked
Message-ID<rfsmg-3ve-65@gated-at.bofh.it>
In reply to#1362563
From: Vladimir Davydov <vdavydov@virtuozzo.com>

Entries are only added/removed from oom_reaper_list at head so we can use
a single linked list and hence save a word in task_struct.

Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/sched.h |  2 +-
 mm/oom_kill.c         | 15 +++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index acb480b581e3..d118445a332e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1841,7 +1841,7 @@ struct task_struct {
 #endif
 	int pagefault_disabled;
 #ifdef CONFIG_MMU
-	struct list_head oom_reaper_list;
+	struct task_struct *oom_reaper_list;
 #endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b38a648558f9..af75260f32c3 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -417,7 +417,7 @@ bool oom_killer_disabled __read_mostly;
  */
 static struct task_struct *oom_reaper_th;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
-static LIST_HEAD(oom_reaper_list);
+static struct task_struct *oom_reaper_list;
 static DEFINE_SPINLOCK(oom_reaper_lock);
 
 
@@ -527,13 +527,11 @@ static int oom_reaper(void *unused)
 	while (true) {
 		struct task_struct *tsk = NULL;
 
-		wait_event_freezable(oom_reaper_wait,
-				     (!list_empty(&oom_reaper_list)));
+		wait_event_freezable(oom_reaper_wait, oom_reaper_list != NULL);
 		spin_lock(&oom_reaper_lock);
-		if (!list_empty(&oom_reaper_list)) {
-			tsk = list_first_entry(&oom_reaper_list,
-					struct task_struct, oom_reaper_list);
-			list_del(&tsk->oom_reaper_list);
+		if (oom_reaper_list != NULL) {
+			tsk = oom_reaper_list;
+			oom_reaper_list = tsk->oom_reaper_list;
 		}
 		spin_unlock(&oom_reaper_lock);
 
@@ -552,7 +550,8 @@ static void wake_oom_reaper(struct task_struct *tsk)
 	get_task_struct(tsk);
 
 	spin_lock(&oom_reaper_lock);
-	list_add(&tsk->oom_reaper_list, &oom_reaper_list);
+	tsk->oom_reaper_list = oom_reaper_list;
+	oom_reaper_list = tsk;
 	spin_unlock(&oom_reaper_lock);
 	wake_up(&oom_reaper_wait);
 }
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1362582 — [PATCH 6/9] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-22 12:10 +0100
Subject[PATCH 6/9] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task
Message-ID<rfsmg-3ve-75@gated-at.bofh.it>
In reply to#1362563
From: Michal Hocko <mhocko@suse.com>

Tetsuo has reported that oom_kill_allocating_task=1 will cause
oom_reaper_list corruption because oom_kill_process doesn't follow
standard OOM exclusion (aka ignores TIF_MEMDIE) and allows to enqueue
the same task multiple times - e.g. by sacrificing the same child
multiple times.

This patch fixes the issue by introducing a new MMF_OOM_KILLED mm flag
which is set in oom_kill_process atomically and oom reaper is disabled
if the flag was already set.

Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/sched.h | 2 ++
 mm/oom_kill.c         | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index bc5867296f7b..acb480b581e3 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -511,6 +511,8 @@ static inline int get_dumpable(struct mm_struct *mm)
 #define MMF_HAS_UPROBES		19	/* has uprobes */
 #define MMF_RECALC_UPROBES	20	/* MMF_HAS_UPROBES can be wrong */
 
+#define MMF_OOM_KILLED		21	/* OOM killer has chosen this mm */
+
 #define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK)
 
 struct sighand_struct {
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 8e0bd279135f..b38a648558f9 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -679,7 +679,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	unsigned int victim_points = 0;
 	static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
 					      DEFAULT_RATELIMIT_BURST);
-	bool can_oom_reap = true;
+	bool can_oom_reap;
 
 	/*
 	 * If the task is already exiting, don't alarm the sysadmin or kill
@@ -741,6 +741,10 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	/* Get a reference to safely compare mm after task_unlock(victim) */
 	mm = victim->mm;
 	atomic_inc(&mm->mm_count);
+
+	/* Make sure we do not try to oom reap the mm multiple times */
+	can_oom_reap = !test_and_set_bit(MMF_OOM_KILLED, &mm->flags);
+
 	/*
 	 * We should send SIGKILL before setting TIF_MEMDIE in order to prevent
 	 * the OOM victim from depleting the memory reserves from the user
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1362583 — [PATCH 4/9] mm, oom_reaper: report success/failure

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-22 12:10 +0100
Subject[PATCH 4/9] mm, oom_reaper: report success/failure
Message-ID<rfsmg-3ve-81@gated-at.bofh.it>
In reply to#1362563
From: Michal Hocko <mhocko@suse.com>

Inform about the successful/failed oom_reaper attempts and dump all the
held locks to tell us more who is blocking the progress.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/oom_kill.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 2830b1c6483e..e627ce235e38 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -419,6 +419,7 @@ static struct task_struct *oom_reaper_th;
 static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
 
+#define K(x) ((x) << (PAGE_SHIFT-10))
 static bool __oom_reap_task(struct task_struct *tsk)
 {
 	struct mmu_gather tlb;
@@ -479,6 +480,11 @@ static bool __oom_reap_task(struct task_struct *tsk)
 					 &details);
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
+	pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
+			task_pid_nr(tsk), tsk->comm,
+			K(get_mm_counter(mm, MM_ANONPAGES)),
+			K(get_mm_counter(mm, MM_FILEPAGES)),
+			K(get_mm_counter(mm, MM_SHMEMPAGES)));
 	up_read(&mm->mmap_sem);
 
 	/*
@@ -495,14 +501,21 @@ static bool __oom_reap_task(struct task_struct *tsk)
 	return ret;
 }
 
+#define MAX_OOM_REAP_RETRIES 10
 static void oom_reap_task(struct task_struct *tsk)
 {
 	int attempts = 0;
 
 	/* Retry the down_read_trylock(mmap_sem) a few times */
-	while (attempts++ < 10 && !__oom_reap_task(tsk))
+	while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task(tsk))
 		schedule_timeout_idle(HZ/10);
 
+	if (attempts > MAX_OOM_REAP_RETRIES) {
+		pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
+				task_pid_nr(tsk), tsk->comm);
+		debug_show_all_locks();
+	}
+
 	/* Drop a reference taken by wake_oom_reaper */
 	put_task_struct(tsk);
 }
@@ -649,7 +662,6 @@ static bool process_shares_mm(struct task_struct *p, struct mm_struct *mm)
 	return false;
 }
 
-#define K(x) ((x) << (PAGE_SHIFT-10))
 /*
  * Must be called while holding a reference to p, which will be released upon
  * returning.
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]


#1363036

FromDavid Rientjes <rientjes@google.com>
Date2016-03-22 23:10 +0100
Message-ID<rfCEW-2qy-1@gated-at.bofh.it>
In reply to#1362563
On Tue, 22 Mar 2016, Michal Hocko wrote:

> Hi,
> I am reposting the whole patchset on top of the current Linus tree which should
> already contain big pile of Andrew's mm patches. This should serve an easier
> reviewability and I also hope that this core part of the work can go to 4.6.
> 
> The previous version was posted here [1] Hugh and David have suggested to
> drop [2] because the munlock path currently depends on the page lock and
> it is better if the initial version was conservative and prevent from
> any potential lockups even though it is not clear whether they are real
> - nobody has seen oom_reaper stuck on the page lock AFAICK. Me or Hugh
> will have a look and try to make the munlock path not depend on the page
> lock as a follow up work.
> 
> Apart from that the feedback revealed one bug for a very unusual
> configuration (sysctl_oom_kill_allocating_task) and that has been fixed
> by patch 8 and one potential mis interaction with the pm freezer fixed by
> patch 7.
> 
> I think the current code base is already very useful for many situations.
> The rest of the feedback was mostly about potential enhancements of the
> current code which I would really prefer to build on top of the current
> series. I plan to finish my mmap_sem killable for write in the upcoming
> release cycle and hopefully have it merged in the next merge window.
> I believe more extensions will follow.
> 
> This code has been sitting in the mmotm (thus linux-next) for a while.
> Are there any fundamental objections to have this part merged in this
> merge window?
> 

Tetsuo, have you been able to run your previous test cases on top of this 
version and do you have any concerns about it or possible extensions that 
could be made?

[toc] | [prev] | [next] | [standalone]


#1363358

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-03-23 12:30 +0100
Message-ID<rfP99-2DP-25@gated-at.bofh.it>
In reply to#1363036
David Rientjes wrote:
> On Tue, 22 Mar 2016, Michal Hocko wrote:
> 
> > Hi,
> > I am reposting the whole patchset on top of the current Linus tree which should
> > already contain big pile of Andrew's mm patches. This should serve an easier
> > reviewability and I also hope that this core part of the work can go to 4.6.
> > 
> > The previous version was posted here [1] Hugh and David have suggested to
> > drop [2] because the munlock path currently depends on the page lock and
> > it is better if the initial version was conservative and prevent from
> > any potential lockups even though it is not clear whether they are real
> > - nobody has seen oom_reaper stuck on the page lock AFAICK. Me or Hugh
> > will have a look and try to make the munlock path not depend on the page
> > lock as a follow up work.
> > 
> > Apart from that the feedback revealed one bug for a very unusual
> > configuration (sysctl_oom_kill_allocating_task) and that has been fixed
> > by patch 8 and one potential mis interaction with the pm freezer fixed by
> > patch 7.
> > 
> > I think the current code base is already very useful for many situations.
> > The rest of the feedback was mostly about potential enhancements of the
> > current code which I would really prefer to build on top of the current
> > series. I plan to finish my mmap_sem killable for write in the upcoming
> > release cycle and hopefully have it merged in the next merge window.
> > I believe more extensions will follow.
> > 
> > This code has been sitting in the mmotm (thus linux-next) for a while.
> > Are there any fundamental objections to have this part merged in this
> > merge window?
> > 
> 
> Tetsuo, have you been able to run your previous test cases on top of this 
> version and do you have any concerns about it or possible extensions that 
> could be made?
> 

I think [PATCH 3/9] [PATCH 4/9] [PATCH 8/9] will be mostly reverted.
My concerns and possible extensions are explained in

    Re: [PATCH 6/5] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task
    http://lkml.kernel.org/r/201603152015.JAE86937.VFOLtQFOFJOSHM@I-love.SAKURA.ne.jp

. Regarding "[PATCH 4/9] mm, oom_reaper: report success/failure",
debug_show_all_locks() may not be safe

    commit 856848737bd944c1 "lockdep: fix debug_show_all_locks()"
    commit 82a1fcb90287052a "softlockup: automatically detect hung TASK_UNINTERRUPTIBLE tasks"

and showing traces might be more useful.
(A discussion for making printk() completely async is in progress.)

But we don't have time to update this series before merge window for 4.6 closes.
We want to send current patchset as is for now, don't we? So, please go ahead.



My other concerns about OOM handling:

  Change TIF_MEMDIE strategy from per a thread to per a signal_struct.

    [PATCH] mm,oom: Set TIF_MEMDIE on all OOM-killed threads.
    http://lkml.kernel.org/r/1458529634-5951-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp

  Found a bug in too_many_isolated() assumption.

    How to handle infinite too_many_isolated() loop (for OOM detection rework v4) ?
    http://lkml.kernel.org/r/201602092349.ACG81273.OSVtMJQHLOFOFF@I-love.SAKURA.ne.jp

  Waiting for a patch to be merged.

    [PATCH] mm,writeback: Don't use memory reserves for wb_start_writeback
    http://lkml.kernel.org/r/20160318134230.GC30225@dhcp22.suse.cz

  And, kmallocwd, __GFP_KILLABLE, and timeout (or something finite one) for TIF_MEMDIE.

[toc] | [prev] | [next] | [standalone]


#1363385

FromMichal Hocko <mhocko@kernel.org>
Date2016-03-23 13:10 +0100
Message-ID<rfPLQ-3a6-23@gated-at.bofh.it>
In reply to#1363358
On Wed 23-03-16 20:11:35, Tetsuo Handa wrote:
> David Rientjes wrote:
[...]
> > Tetsuo, have you been able to run your previous test cases on top of this 
> > version and do you have any concerns about it or possible extensions that 
> > could be made?
> > 
> 
> I think [PATCH 3/9] [PATCH 4/9] [PATCH 8/9] will be mostly reverted.
> My concerns and possible extensions are explained in
> 
>     Re: [PATCH 6/5] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task
>     http://lkml.kernel.org/r/201603152015.JAE86937.VFOLtQFOFJOSHM@I-love.SAKURA.ne.jp

I believe issues you have raised there are a matter for further
discussion as they are potential improvements of the existing
functionality rather than fixing a regression of the current code.

> . Regarding "[PATCH 4/9] mm, oom_reaper: report success/failure",
> debug_show_all_locks() may not be safe
> 
>     commit 856848737bd944c1 "lockdep: fix debug_show_all_locks()"
>     commit 82a1fcb90287052a "softlockup: automatically detect hung TASK_UNINTERRUPTIBLE tasks"

Let me ask again. What exactly is unsafe about calling
debug_show_all_locks here? It is true that 856848737bd944c1 has
changed debug_show_all_locks to ignore running tasks which limits
this functionality to some degree but I still think this might be
useful. Proposed alternatives were way too verbose and complex on its
own. This is something to be further discussed as well, though.

> and showing traces might be more useful.
> (A discussion for making printk() completely async is in progress.)
> 
> But we don't have time to update this series before merge window for 4.6 closes.
> We want to send current patchset as is for now, don't we? So, please go ahead.

I am happy that we are on the same patch here.

> My other concerns about OOM handling:

Let's stick to oom reaper here, please.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web