Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1463417 > unrolled thread

Re: [PATCH v6 05/11] mm, compaction: add the ultimate direct compaction priority

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v6 05/11] mm, compaction: add the ultimate direct  compaction priority Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-08-16 08:00 +0200
    Re: [PATCH v6 05/11] mm, compaction: add the ultimate direct  compaction priority Vlastimil Babka <vbabka@suse.cz> - 2016-08-18 14:30 +0200

#1463417 — Re: [PATCH v6 05/11] mm, compaction: add the ultimate direct compaction priority

On Wed, Aug 10, 2016 at 11:12:20AM +0200, Vlastimil Babka wrote:
> During reclaim/compaction loop, it's desirable to get a final answer from
> unsuccessful compaction so we can either fail the allocation or invoke the OOM
> killer. However, heuristics such as deferred compaction or pageblock skip bits
> can cause compaction to skip parts or whole zones and lead to premature OOM's,
> failures or excessive reclaim/compaction retries.
> 
> To remedy this, we introduce a new direct compaction priority called
> COMPACT_PRIO_SYNC_FULL, which instructs direct compaction to:
> 
> - ignore deferred compaction status for a zone
> - ignore pageblock skip hints
> - ignore cached scanner positions and scan the whole zone
> 
> The new priority should get eventually picked up by should_compact_retry() and
> this should improve success rates for costly allocations using __GFP_REPEAT,
> such as hugetlbfs allocations, and reduce some corner-case OOM's for non-costly
> allocations.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/compaction.h | 3 ++-
>  mm/compaction.c            | 5 ++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index e88c037afe47..a1fba9994728 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -6,8 +6,9 @@
>   * Lower value means higher priority, analogically to reclaim priority.
>   */
>  enum compact_priority {
> +	COMPACT_PRIO_SYNC_FULL,
> +	MIN_COMPACT_PRIORITY = COMPACT_PRIO_SYNC_FULL,
>  	COMPACT_PRIO_SYNC_LIGHT,
> -	MIN_COMPACT_PRIORITY = COMPACT_PRIO_SYNC_LIGHT,
>  	DEF_COMPACT_PRIORITY = COMPACT_PRIO_SYNC_LIGHT,
>  	COMPACT_PRIO_ASYNC,
>  	INIT_COMPACT_PRIORITY = COMPACT_PRIO_ASYNC
> diff --git a/mm/compaction.c b/mm/compaction.c
> index a144f58f7193..ae4f40afcca1 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1644,6 +1644,8 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
>  		.alloc_flags = alloc_flags,
>  		.classzone_idx = classzone_idx,
>  		.direct_compaction = true,
> +		.whole_zone = (prio == COMPACT_PRIO_SYNC_FULL),
> +		.ignore_skip_hint = (prio == COMPACT_PRIO_SYNC_FULL)
>  	};
>  	INIT_LIST_HEAD(&cc.freepages);
>  	INIT_LIST_HEAD(&cc.migratepages);
> @@ -1689,7 +1691,8 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
>  								ac->nodemask) {
>  		enum compact_result status;
>  
> -		if (compaction_deferred(zone, order)) {
> +		if (prio > COMPACT_PRIO_SYNC_FULL
> +					&& compaction_deferred(zone, order)) {
>  			rc = max_t(enum compact_result, COMPACT_DEFERRED, rc);
>  			continue;

Could we provide prio to compaction_deferred() and do the decision in
that that function?

BTW, in kcompactd, compaction_deferred() is checked but
.ignore_skip_hint=true. Is there any reason? If we can remove
compaction_deferred() for kcompactd, we can check .ignore_skip_hint
to determine if defer is needed or not.

Thanks.

[toc] | [next] | [standalone]

#1465219

On 08/16/2016 07:58 AM, Joonsoo Kim wrote:
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -1644,6 +1644,8 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
>>  		.alloc_flags = alloc_flags,
>>  		.classzone_idx = classzone_idx,
>>  		.direct_compaction = true,
>> +		.whole_zone = (prio == COMPACT_PRIO_SYNC_FULL),
>> +		.ignore_skip_hint = (prio == COMPACT_PRIO_SYNC_FULL)
>>  	};
>>  	INIT_LIST_HEAD(&cc.freepages);
>>  	INIT_LIST_HEAD(&cc.migratepages);
>> @@ -1689,7 +1691,8 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
>>  								ac->nodemask) {
>>  		enum compact_result status;
>>
>> -		if (compaction_deferred(zone, order)) {
>> +		if (prio > COMPACT_PRIO_SYNC_FULL
>> +					&& compaction_deferred(zone, order)) {
>>  			rc = max_t(enum compact_result, COMPACT_DEFERRED, rc);
>>  			continue;
>
> Could we provide prio to compaction_deferred() and do the decision in
> that that function?
>
> BTW, in kcompactd, compaction_deferred() is checked but
> .ignore_skip_hint=true. Is there any reason? If we can remove
> compaction_deferred() for kcompactd, we can check .ignore_skip_hint
> to determine if defer is needed or not.

I don't want to change kcompactd right now, as the current series seems 
to help against premature OOMs. But I'll revisit it later.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web