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

From: Andrew Morton
Date: Tue Apr 05 2016 - 19:58:33 EST


On Tue, 5 Apr 2016 13:25:31 +0200 Michal Hocko <mhocko@xxxxxxxxxx> wrote:

> From: Michal Hocko <mhocko@xxxxxxxx>
>
> Compaction can provide a wild variation of feedback to the caller. Many
> of them are implementation specific and the caller of the compaction
> (especially the page allocator) shouldn't be bound to specifics of the
> current implementation.
>
> This patch abstracts the feedback into three basic types:
> - compaction_made_progress - compaction was active and made some
> progress.
> - compaction_failed - compaction failed and further attempts to
> invoke it would most probably fail and therefore it is not
> worth retrying
> - compaction_withdrawn - compaction wasn't invoked for an
> implementation specific reasons. In the current implementation
> it means that the compaction was deferred, contended or the
> page scanners met too early without any progress. Retrying is
> still worthwhile.
>
> ...
>
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3362,25 +3362,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> if (page)
> goto got_pg;
>
> - /* Checks for THP-specific high-order allocations */
> - 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;