linux-kernel - Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to vmpressure

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, 2 Feb 2017 11:48:09 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Vinayak Menon <vinmenon@...eaurora.org>
Cc:     akpm@...ux-foundation.org, hannes@...xchg.org,
        mgorman@...hsingularity.net, vbabka@...e.cz, riel@...hat.com,
        vdavydov.dev@...il.com, anton.vorontsov@...aro.org,
        minchan@...nel.org, shashim@...eaurora.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to
 vmpressure

On Thu 02-02-17 11:44:22, Michal Hocko wrote:
> On Tue 31-01-17 14:32:08, Vinayak Menon wrote:
> > During global reclaim, the nr_reclaimed passed to vmpressure
> > includes the pages reclaimed from slab. But the corresponding
> > scanned slab pages is not passed. This can cause total reclaimed
> > pages to be greater than scanned, causing an unsigned underflow
> > in vmpressure resulting in a critical event being sent to root
> > cgroup. So do not consider reclaimed slab pages for vmpressure
> > calculation. The reclaimed pages from slab can be excluded because
> > the freeing of a page by slab shrinking depends on each slab's
> > object population, making the cost model (i.e. scan:free) different
> > from that of LRU.
> 
> This might be true but what happens if the slab reclaim contributes
> significantly to the overal reclaim? This would be quite rare but not
> impossible.
> 
> I am wondering why we cannot simply make cap nr_reclaimed to nr_scanned
> and be done with this all? Sure it will be imprecise but the same will
> be true with this approach.

In other words something as "beautiful" as the following:
diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index 149fdf6c5c56..abea42817dd0 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -236,6 +236,15 @@ void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree,
 		return;
 
 	/*
+	 * Due to accounting issues - e.g. THP contributing 1 to scanned but
+	 * potentially much more to reclaimed or SLAB pages not contributing
+	 * to scanned at all - we have to skew reclaimed to prevent from
+	 * wrong pressure levels due to overflows.
+	 */
+	if (reclaimed > scanned)
+		reclaimed = scanned;
+
+	/*
 	 * If we got here with no pages scanned, then that is an indicator
 	 * that reclaimer was unable to find any shrinkable LRUs at the
 	 * current scanning depth. But it does not mean that we should
-- 
Michal Hocko
SUSE Labs