linux-kernel - Re: [PATCH 09/11] mm, compaction: Abstract compaction feedback to helpers

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: <alpine.LSU.2.11.1604051727150.7348@eggly.anvils>
Date:	Tue, 5 Apr 2016 17:55:39 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
cc:	Michal Hocko <mhocko@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	Mel Gorman <mgorman@...e.de>,
	David Rientjes <rientjes@...gle.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Joonsoo Kim <js1304@...il.com>,
	Hillf Danton <hillf.zj@...baba-inc.com>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>,
	Michal Hocko <mhocko@...e.com>, Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH 09/11] mm, compaction: Abstract compaction feedback to
 helpers

On Tue, 5 Apr 2016, Andrew Morton wrote:
> On Tue,  5 Apr 2016 13:25:31 +0200 Michal Hocko <mhocko@...nel.org> wrote:
> > -	if (is_thp_gfp_mask(gfp_mask)) {
> > -		/*
> > -		 * If compaction is deferred for high-order allocations, it is
> > -		 * because sync compaction recently failed. If this is the case
> > -		 * and the caller requested a THP allocation, we do not want
> > -		 * to heavily disrupt the system, so we fail the allocation
> > -		 * instead of entering direct reclaim.
> > -		 */
> > -		if (compact_result == COMPACT_DEFERRED)
> > -			goto nopage;
> > -
> > -		/*
> > -		 * Compaction is contended so rather back off than cause
> > -		 * excessive stalls.
> > -		 */
> > -		if(compact_result == COMPACT_CONTENDED)
> > -			goto nopage;
> > -	}
> > +	/*
> > +	 * Checks for THP-specific high-order allocations and back off
> > +	 * if the the compaction backed off
> > +	 */
> > +	if (is_thp_gfp_mask(gfp_mask) && compaction_withdrawn(compact_result))
> > +		goto nopage;
> 
> This change smashed into Hugh's "huge tmpfs: shmem_huge_gfpmask and
> shmem_recovery_gfpmask".
> 
> I ended up doing this:
> 
> 	/* Checks for THP-specific high-order allocations */
> 	if (!is_thp_allocation(gfp_mask, order))
> 		migration_mode = MIGRATE_SYNC_LIGHT;
> 
> 	/*
> 	 * Checks for THP-specific high-order allocations and back off
> 	 * if the the compaction backed off
> 	 */
> 	if (is_thp_allocation(gfp_mask) && compaction_withdrawn(compact_result))
> 		goto nopage;

You'll already have found that is_thp_allocation() needs the order too.
But then you had to drop a hunk out of his 10/11 also to fit with mine.

What you've done may be just right, but I haven't had time to digest
Michal's changes yet (and not yet seen what happens to the PF_KTHREAD
distinction), so I think it will probably end up better if you take
his exactly as he tested and posted them, and drop my 30/31 and 31/31
for now - I can resubmit them (or maybe drop 30 altogether) after I've
pondered and tested a little on top of Michal's.

Huge tmpfs got along fine for many months without 30/31 and 31/31: 30
is just for experimentation, and 31 to reduce the compaction stalls we
saw under some loads.  Maybe I'll find that Michal's rework has changed
the balance there anyway, and something else or nothing at all needed.

(The gfp_mask stuff was very confusing, and it's painful for me, how
~__GFP_KSWAPD_RECLAIM gets used as a secret password to say "THP" and
how to angle compaction - or maybe it's all more straightforward now.)

Many thanks for giving us both this quick exposure!

Hugh