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

[Hugh Dickins]

On Tue, 5 Apr 2016, Andrew Morton wrote:
> On Tue,  5 Apr 2016 13:25:31 +0200 Michal Hocko <mhocko@xxxxxxxxxx> 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