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

Groups > linux.kernel > #1730100 > unrolled thread

Re: [patch 1/2] mm, compaction: kcompactd should not ignore pageblock skip

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 1/2] mm, compaction: kcompactd should not ignore pageblock  skip David Rientjes <rientjes@google.com> - 2017-09-11 03:10 +0200
    Re: [patch 1/2] mm, compaction: kcompactd should not ignore pageblock  skip Vlastimil Babka <vbabka@suse.cz> - 2017-09-11 08:40 +0200
      Re: [patch 1/2] mm, compaction: kcompactd should not ignore pageblock  skip David Rientjes <rientjes@google.com> - 2017-09-11 23:20 +0200

#1730100 — Re: [patch 1/2] mm, compaction: kcompactd should not ignore pageblock skip

On Wed, 23 Aug 2017, Vlastimil Babka wrote:

> On 08/16/2017 01:39 AM, David Rientjes wrote:
> > Kcompactd is needlessly ignoring pageblock skip information.  It is doing
> > MIGRATE_SYNC_LIGHT compaction, which is no more powerful than
> > MIGRATE_SYNC compaction.
> > 
> > If compaction recently failed to isolate memory from a set of pageblocks,
> > there is nothing to indicate that kcompactd will be able to do so, or
> > that it is beneficial from attempting to isolate memory.
> > 
> > Use the pageblock skip hint to avoid rescanning pageblocks needlessly
> > until that information is reset.
> > 
> > Signed-off-by: David Rientjes <rientjes@google.com>
> 
> It would be much better if patches like this were accompanied by some
> numbers.
> 

The numbers were from https://marc.info/?l=linux-mm&m=150231232707999 
where very large amounts (>90% of system RAM) were hugetlb pages.  We can 
supplement this changelog with the following if it helps:

"""
Currently, kcompactd ignores pageblock skip information that can become 
useful if it is known that memory should not be considered by both the 
migration and freeing scanners.  Abundant hugetlb memory is a good example 
of memory that is needlessly (and expensively) scanned since the hugepage 
order normally matches the pageblock order.

For example, on a sysctl with very large amounts of memory reserved by the 
hugetlb subsystem:

compact_migrate_scanned 2931254031294 
compact_free_scanned    102707804816705 
compact_isolated        1309145254 

Kcompactd ends up successfully isolating ~0.0012% of memory that is 
scans (the above does not involve direct compaction).

A follow-up change will set the pageblock skip for this memory since it is 
never useful for either scanner.
"""

> Also there's now a danger that in cases where there's no direct
> compaction happening (just kcompactd), nothing will ever call
> __reset_isolation_suitable().
> 

I'm not sure that is helpful in a context where no high-order memory can 
call direct compaction that kcompactd needlessly scanning the same memory 
over and over is beneficial.

> > ---
> >  mm/compaction.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -1927,9 +1927,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
> >  		.total_free_scanned = 0,
> >  		.classzone_idx = pgdat->kcompactd_classzone_idx,
> >  		.mode = MIGRATE_SYNC_LIGHT,
> > -		.ignore_skip_hint = true,
> > +		.ignore_skip_hint = false,
> >  		.gfp_mask = GFP_KERNEL,
> > -
> >  	};
> >  	trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order,
> >  							cc.classzone_idx);
> > 
> 
> 

[toc] | [next] | [standalone]

#1730161

On 09/11/2017 03:07 AM, David Rientjes wrote:
> On Wed, 23 Aug 2017, Vlastimil Babka wrote:
> 
>> On 08/16/2017 01:39 AM, David Rientjes wrote:
>>> Kcompactd is needlessly ignoring pageblock skip information.  It is doing
>>> MIGRATE_SYNC_LIGHT compaction, which is no more powerful than
>>> MIGRATE_SYNC compaction.
>>>
>>> If compaction recently failed to isolate memory from a set of pageblocks,
>>> there is nothing to indicate that kcompactd will be able to do so, or
>>> that it is beneficial from attempting to isolate memory.
>>>
>>> Use the pageblock skip hint to avoid rescanning pageblocks needlessly
>>> until that information is reset.
>>>
>>> Signed-off-by: David Rientjes <rientjes@google.com>
>>
>> It would be much better if patches like this were accompanied by some
>> numbers.
>>
> 
> The numbers were from https://marc.info/?l=linux-mm&m=150231232707999 
> where very large amounts (>90% of system RAM) were hugetlb pages.  We can 
> supplement this changelog with the following if it helps:
> 
> """
> Currently, kcompactd ignores pageblock skip information that can become 
> useful if it is known that memory should not be considered by both the 
> migration and freeing scanners.  Abundant hugetlb memory is a good example 
> of memory that is needlessly (and expensively) scanned since the hugepage 
> order normally matches the pageblock order.
> 
> For example, on a sysctl with very large amounts of memory reserved by the 
> hugetlb subsystem:
> 
> compact_migrate_scanned 2931254031294 
> compact_free_scanned    102707804816705 
> compact_isolated        1309145254 
> 
> Kcompactd ends up successfully isolating ~0.0012% of memory that is 
> scans (the above does not involve direct compaction).

Yeah it would be nice to have the numbers also after the patch and to
see also effect on the kcompactd success rate.

> A follow-up change will set the pageblock skip for this memory since it is 
> never useful for either scanner.
> """
> 
>> Also there's now a danger that in cases where there's no direct
>> compaction happening (just kcompactd), nothing will ever call
>> __reset_isolation_suitable().
>>
> 
> I'm not sure that is helpful in a context where no high-order memory can 
> call direct compaction that kcompactd needlessly scanning the same memory 
> over and over is beneficial.

The point is that if it becomes beneficial again, we won't know as there
will be still be skip bits.

>>> ---
>>>  mm/compaction.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>> --- a/mm/compaction.c
>>> +++ b/mm/compaction.c
>>> @@ -1927,9 +1927,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>>>  		.total_free_scanned = 0,
>>>  		.classzone_idx = pgdat->kcompactd_classzone_idx,
>>>  		.mode = MIGRATE_SYNC_LIGHT,
>>> -		.ignore_skip_hint = true,
>>> +		.ignore_skip_hint = false,
>>>  		.gfp_mask = GFP_KERNEL,
>>> -
>>>  	};
>>>  	trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order,
>>>  							cc.classzone_idx);
>>>
>>
>>

[toc] | [prev] | [next] | [standalone]

#1730559

On Mon, 11 Sep 2017, Vlastimil Babka wrote:

> > A follow-up change will set the pageblock skip for this memory since it is 
> > never useful for either scanner.
> > """
> > 
> >> Also there's now a danger that in cases where there's no direct
> >> compaction happening (just kcompactd), nothing will ever call
> >> __reset_isolation_suitable().
> >>
> > 
> > I'm not sure that is helpful in a context where no high-order memory can 
> > call direct compaction that kcompactd needlessly scanning the same memory 
> > over and over is beneficial.
> 
> The point is that if it becomes beneficial again, we won't know as there
> will be still be skip bits.
> 

Why is kcompactd_do_work() not sometimes doing 
__reset_isolation_suitable() in the first place, if only to reset the 
per-zone migration and freeing scanner cached pfns?  It seems fragile to 
rely on other threads doing direct compaction to reset the per-zone state 
of compaction.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web