Re: [RFC 07/13] mm, compaction: introduce direct compaction priority

Michal Hocko
2016-05-13T12:37:53Z

On Tue 10-05-16 09:35:57, Vlastimil Babka wrote:
> In the context of direct compaction, for some types of allocations we would
> like the compaction to either succeed or definitely fail while trying as hard
> as possible. Current async/sync_light migration mode is insufficient, as there
> are heuristics such as caching scanner positions, marking pageblocks as
> unsuitable or deferring compaction for a zone. At least the final compaction
> attempt should be able to override these heuristics.
>
> To communicate how hard compaction should try, we replace migration mode with
> a new enum compact_priority and change the relevant function signatures. In
> compact_zone_order() where struct compact_control is constructed, the priority
> is mapped to suitable control flags. This patch itself has no functional
> change, as the current priority levels are mapped back to the same migration
> modes as before. Expanding them will be done next.
>
> Note that !CONFIG_COMPACTION variant of try_to_compact_pages() is removed, as
> the only caller exists under CONFIG_COMPACTION.

Your s-o-b is missing

Anyway I like the idea. The migration_mode felt really weird. It exposes
an internal detail of the compaction code which should have no business
in the allocator path.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
> include/linux/compaction.h | 18 +++++++++---------
> mm/compaction.c | 14 ++++++++------
> mm/page_alloc.c | 27 +++++++++++++--------------
> 3 files changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index 4ba90e74969c..900d181ff1b0 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -1,6 +1,14 @@
> #ifndef _LINUX_COMPACTION_H
> #define _LINUX_COMPACTION_H
>
> +// TODO: lower value means higher priority to match reclaim, makes sense?

Yes this makes sense to me.

> +enum compact_priority {

enums might be tricky but I guess it should work ok here. I would just
add

COMPACT_MIN_PRIO,
> + COMPACT_PRIO_SYNC_LIGHT = COMPACT_MIN_PRIO,
> + DEF_COMPACT_PRIORITY = COMPACT_PRIO_SYNC_LIGHT,
> + COMPACT_PRIO_ASYNC,
> + INIT_COMPACT_PRIORITY = COMPACT_PRIO_ASYNC
> +};
> +

to make an implementation independent lowest priority.

[...]

> @@ -3269,11 +3269,11 @@ should_compact_retry(struct alloc_context *ac, int order, int alloc_flags,
> /*
> * compaction considers all the zone as desperately out of memory
> * so it doesn't really make much sense to retry except when the
> - * failure could be caused by weak migration mode.
> + * failure could be caused by insufficient priority
> */
> if (compaction_failed(compact_result)) {
> - if (*migrate_mode == MIGRATE_ASYNC) {
> - *migrate_mode = MIGRATE_SYNC_LIGHT;
> + if (*compact_priority > 0) {

if (*compact_priority > COMPACT_MIN_PRIO)

> + (*compact_priority)--;
> return true;
> }
> return false;
--
Michal Hocko
SUSE Labs