linux-kernel - Re: [PATCH 1/2] mm, oom: introduce oom reaper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Dec 2015 14:02:24 +0100
From:	Michal Hocko <mhocko@...nel.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Mel Gorman <mgorman@...e.de>,
	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	David Rientjes <rientjes@...gle.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Hugh Dickins <hughd@...gle.com>,
	Andrea Argangeli <andrea@...nel.org>,
	Rik van Riel <riel@...hat.com>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mm, oom: introduce oom reaper

On Wed 16-12-15 16:50:35, Andrew Morton wrote:
> On Tue, 15 Dec 2015 19:36:15 +0100 Michal Hocko <mhocko@...nel.org> wrote:
[...]
> > +static void oom_reap_vmas(struct mm_struct *mm)
> > +{
> > +	int attempts = 0;
> > +
> > +	while (attempts++ < 10 && !__oom_reap_vmas(mm))
> > +		schedule_timeout(HZ/10);
> 
> schedule_timeout() in state TASK_RUNNING doesn't do anything.  Use
> msleep() or msleep_interruptible().  I can't decide which is more
> appropriate - it only affects the load average display.

Ups. You are right. I will go with msleep_interruptible(100).
 
> Which prompts the obvious question: as the no-operativeness of this
> call wasn't noticed in testing, why do we have it there...

Well, the idea was that an interfering mmap_sem operation which holds
it for write might block us for a short time period - e.g. when not
depending on an allocation or accessing the memory reserves helps
to progress the allocation. If the holder of the semaphore is stuck
then the retry is pointless. On the other hand the retry shouldn't be
harmful. All in all this is just a heuristic and we do not depend on
it. I guess we can drop it and nobody would actually notice. Let me know
if you prefer that and I will respin the patch.


> I guess it means that the __oom_reap_vmas() success rate is nice anud
> high ;)

I had a debugging trace_printks around this and there were no reties
during my testing so I was probably lucky to not trigger the mmap_sem
contention.
---
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 48025a21f8c4..f53f87cfd899 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -469,7 +469,7 @@ static void oom_reap_vmas(struct mm_struct *mm)
 	int attempts = 0;
 
 	while (attempts++ < 10 && !__oom_reap_vmas(mm))
-		schedule_timeout(HZ/10);
+		msleep_interruptible(100);
 
 	/* Drop a reference taken by wake_oom_reaper */
 	mmdrop(mm);

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/