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 <mho...@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

Reply via email to