linux-kernel - Re: [patch 2/8] mm: vmscan: disregard swappiness shortly before going OOM

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]

Message-ID: <20121213160521.GG21644@dhcp22.suse.cz>
Date:	Thu, 13 Dec 2012 17:05:21 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Johannes Weiner <hannes@...xchg.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Hugh Dickins <hughd@...gle.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch 2/8] mm: vmscan: disregard swappiness shortly before
 going OOM

On Thu 13-12-12 16:29:59, Michal Hocko wrote:
> On Thu 13-12-12 10:34:20, Mel Gorman wrote:
> > On Wed, Dec 12, 2012 at 04:43:34PM -0500, Johannes Weiner wrote:
> > > When a reclaim scanner is doing its final scan before giving up and
> > > there is swap space available, pay no attention to swappiness
> > > preference anymore.  Just swap.
> > > 
> > > Note that this change won't make too big of a difference for general
> > > reclaim: anonymous pages are already force-scanned when there is only
> > > very little file cache left, and there very likely isn't when the
> > > reclaimer enters this final cycle.
> > > 
> > > Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> > 
> > Ok, I see the motivation for your patch but is the block inside still
> > wrong for what you want? After your patch the block looks like this
> > 
> >                 if (sc->priority || noswap) {
> >                         scan >>= sc->priority;
> >                         if (!scan && force_scan)
> >                                 scan = SWAP_CLUSTER_MAX;
> >                         scan = div64_u64(scan * fraction[file], denominator);
> >                 }
> > 
> > if sc->priority == 0 and swappiness==0 then you enter this block but
> > fraction[0] for anonymous pages will also be 0 and because of the ordering
> > of statements there, scan will be
> > 
> > scan = scan * 0 / denominator
> > 
> > so you are still not reclaiming anonymous pages in the swappiness=0
> > case. What did I miss?
> 
> Yes, now that you have mentioned that I realized that it really doesn't
> make any sense. fraction[0] is _always_ 0 for swappiness==0. So we just
> made a bigger pressure on file LRUs. So this sounds like a misuse of the
> swappiness. This all has been introduced with fe35004f (mm: avoid
> swapping out with swappiness==0).
> 
> I think that removing swappiness check make sense but I am not sure it
> does what the changelog says. It should have said that checking
> swappiness doesn't make any sense for small LRUs.

Bahh, wait a moment. Now I remember why the check made sense especially
for memcg.
It made "don't swap _at all_ for swappiness==0" for real - you are even
willing to sacrifice OOM. Maybe this is OK for the global case because
noswap would safe you here (assuming that there is no swap if somebody
doesn't want to swap at all and swappiness doesn't play such a big role)
but for memcg you really might want to prevent from swapping - not
everybody has memcg swap extension enabled and swappiness is handy then.
So I am not sure this is actually what we want. Need to think about it.
-- 
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/