lkml.org 
[lkml]   [2015]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 4/5] mm, compaction: allow scanners to start at any pfn within the zone
On 01/20/2015 04:18 PM, Zhang Yanfei wrote:
> Hello Vlastimil
>
> 在 2015/1/19 18:05, Vlastimil Babka 写道:
>> Compaction employs two page scanners - migration scanner isolates pages to be
>> the source of migration, free page scanner isolates pages to be the target of
>> migration. Currently, migration scanner starts at the zone's first pageblock
>> and progresses towards the last one. Free scanner starts at the last pageblock
>> and progresses towards the first one. Within a pageblock, each scanner scans
>> pages from the first to the last one. When the scanners meet within the same
>> pageblock, compaction terminates.
>>
>> One consequence of the current scheme, that turns out to be unfortunate, is
>> that the migration scanner does not encounter the pageblocks which were
>> scanned by the free scanner. In a test with stress-highalloc from mmtests,
>> the scanners were observed to meet around the middle of the zone in first two
>> phases (with background memory pressure) of the test when executed after fresh
>> reboot. On further executions without reboot, the meeting point shifts to
>> roughly third of the zone, and compaction activity as well as allocation
>> success rates deteriorates compared to the run after fresh reboot.
>>
>> It turns out that the deterioration is indeed due to the migration scanner
>> processing only a small part of the zone. Compaction also keeps making this
>> bias worse by its activity - by moving all migratable pages towards end of the
>> zone, the free scanner has to scan a lot of full pageblocks to find more free
>> pages. The beginning of the zone contains pageblocks that have been compacted
>> as much as possible, but the free pages there cannot be further merged into
>> larger orders due to unmovable pages. The rest of the zone might contain more
>> suitable pageblocks, but the migration scanner will not reach them. It also
>> isn't be able to move movable pages out of unmovable pageblocks there, which
>> affects fragmentation.
>>
>> This patch is the first step to remove this bias. It allows the compaction
>> scanners to start at arbitrary pfn (aligned to pageblock for practical
>> purposes), called pivot, within the zone. The migration scanner starts at the
>> exact pfn, the free scanner starts at the pageblock preceding the pivot. The
>> direction of scanning is unaffected, but when the migration scanner reaches
>> the last pageblock of the zone, or the free scanner reaches the first
>> pageblock, they wrap and continue with the first or last pageblock,
>> respectively. Compaction terminates when any of the scanners wrap and both
>> meet within the same pageblock.
>>
>> For easier bisection of potential regressions, this patch always uses the
>> first zone's pfn as the pivot. That means the free scanner immediately wraps
>> to the last pageblock and the operation of scanners is thus unchanged. The
>> actual pivot changing is done by the next patch.
>>
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>
> I read through the whole patch, and you can feel free to add:
>
> Acked-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

Thanks.

> I agree with you and the approach to improve the current scheme. One thing
> I think should be carefully treated is how to avoid migrating back and forth
> since the pivot pfn can be changed. I see patch 5 has introduced a policy to
> change the pivot so we can have a careful observation on it.
>
> (The changes in the patch make the code more difficult to understand now...
> and I just find a tiny mistake, please see below)
>
>> Cc: Minchan Kim <minchan@kernel.org>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>> Cc: Michal Nazarewicz <mina86@mina86.com>
>> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>> Cc: Christoph Lameter <cl@linux.com>
>> Cc: Rik van Riel <riel@redhat.com>
>> Cc: David Rientjes <rientjes@google.com>
>> ---
>> include/linux/mmzone.h | 2 +
>> mm/compaction.c | 204 +++++++++++++++++++++++++++++++++++++++++++------
>> mm/internal.h | 1 +
>> 3 files changed, 182 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 2f0856d..47aa181 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -503,6 +503,8 @@ struct zone {
>> unsigned long percpu_drift_mark;
>>
>> #if defined CONFIG_COMPACTION || defined CONFIG_CMA
>> + /* pfn where compaction scanners have initially started last time */
>> + unsigned long compact_cached_pivot_pfn;
>> /* pfn where compaction free scanner should start */
>> unsigned long compact_cached_free_pfn;
>> /* pfn where async and sync compaction migration scanner should start */
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index 5626220..abae89a 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -123,11 +123,16 @@ static inline bool isolation_suitable(struct compact_control *cc,
>> return !get_pageblock_skip(page);
>> }
>>
>> +/*
>> + * Invalidate cached compaction scanner positions, so that compact_zone()
>> + * will reinitialize them on the next compaction.
>> + */
>> static void reset_cached_positions(struct zone *zone)
>> {
>> - zone->compact_cached_migrate_pfn[0] = zone->zone_start_pfn;
>> - zone->compact_cached_migrate_pfn[1] = zone->zone_start_pfn;
>> - zone->compact_cached_free_pfn = zone_end_pfn(zone);
>> + /* Invalid values are re-initialized in compact_zone */
>> + zone->compact_cached_migrate_pfn[0] = 0;
>> + zone->compact_cached_migrate_pfn[1] = 0;
>> + zone->compact_cached_free_pfn = 0;
>> }
>>
>> /*
>> @@ -172,11 +177,35 @@ void reset_isolation_suitable(pg_data_t *pgdat)
>> /* Only flush if a full compaction finished recently */
>> if (zone->compact_blockskip_flush) {
>> __reset_isolation_suitable(zone);
>> - reset_cached_positions(zone);
>> + reset_cached_positions(zone, false);
>
> The second argument should be in patch 5 instead of here, right? ^-^

Yes, a mistake with some last-minute rebasing :)

> Thanks.
>



\
 
 \ /
  Last update: 2015-01-22 16:41    [W:0.081 / U:0.656 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site