lkml.org 
[lkml]   [2014]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 02/14] mm, compaction: defer each zone individually instead of preferred zone
On 07/29/2014 08:38 AM, Joonsoo Kim wrote:
>> /* Return values for compact_zone() and try_to_compact_pages() */
>> +/* compaction didn't start as it was deferred due to past failures */
>> +#define COMPACT_DEFERRED 0
>> /* compaction didn't start as it was not possible or direct reclaim was more suitable */
>> -#define COMPACT_SKIPPED 0
>> +#define COMPACT_SKIPPED 1
>
> Hello,
>
> This change makes some users of compaction_suitable() failed
> unintentionally, because they assume that COMPACT_SKIPPED is 0.
> Please fix them according to this change.

Oops, good catch. Thanks!

>> @@ -2324,27 +2327,31 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>> order, zonelist, high_zoneidx,
>> alloc_flags & ~ALLOC_NO_WATERMARKS,
>> preferred_zone, classzone_idx, migratetype);
>> +
>> if (page) {
>> - preferred_zone->compact_blockskip_flush = false;
>> - compaction_defer_reset(preferred_zone, order, true);
>> + struct zone *zone = page_zone(page);
>> +
>> + zone->compact_blockskip_flush = false;
>> + compaction_defer_reset(zone, order, true);
>> count_vm_event(COMPACTSUCCESS);
>> return page;
>> }
>>
>> /*
>> + * last_compact_zone is where try_to_compact_pages thought
>> + * allocation should succeed, so it did not defer compaction.
>> + * But now we know that it didn't succeed, so we do the defer.
>> + */
>> + if (last_compact_zone && mode != MIGRATE_ASYNC)
>> + defer_compaction(last_compact_zone, order);
>
> I still don't understand why defer_compaction() is needed here.
> defer_compaction() is intended for not struggling doing compaction on
> the zone where we already have tried compaction and found that it
> isn't suitable for compaction. Allocation failure doesn't tell us
> that we have tried compaction for all the zone range so we shouldn't
> make a decision here to defer compaction on this zone carelessly.

OK I can remove that, it should make the code nicer anyway. I also agree
with the argument "for all the zone range" and I also realized that it's
not (both before and after this patch) really the case. I planned to fix
that in the future, but I can probably do it now.
The plan is to call defer_compaction() only when compaction returned
COMPACT_COMPLETE (and not COMPACT_PARTIAL) as it means the whole zone
was scanned. Otherwise there will be bias towards the beginning of the
zone in the migration scanner - compaction will be deferred half-way and
then cached pfn's might be reset when it restarts, and the rest of the
zone won't be scanned at all.

> Thanks.
>



\
 
 \ /
  Last update: 2014-07-29 12:01    [W:0.177 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site