On 15 Sep 2023, at 5:41, Baolin Wang wrote:
> On 9/13/2023 12:28 AM, Zi Yan wrote:
>> From: Zi Yan <ziy@xxxxxxxxxx>
>>
>> Since compaction code can compact >0 order folios, enable it during the
>> process.
>>
>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>> ---
>> mm/compaction.c | 25 ++++++++++---------------
>> 1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index 4300d877b824..f72af74094de 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -1087,11 +1087,17 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>> if (PageCompound(page) && !cc->alloc_contig) {
>> const unsigned int order = compound_order(page);
>> - if (likely(order <= MAX_ORDER)) {
>> - low_pfn += (1UL << order) - 1;
>> - nr_scanned += (1UL << order) - 1;
>> + /*
>> + * Compacting > pageblock_order pages does not improve
>> + * memory fragmentation. Also skip hugetlbfs pages.
>> + */
>> + if (likely(order >= pageblock_order) || PageHuge(page)) {
>
> IMO, if the compound page order is larger than the requested cc->order, we should also fail the isolation, cause it also does not improve fragmentation, right?
>
Probably yes. I think the reasoning should be since compaction is asking for cc->order,
we should not compacting folios with orders larger than or equal to that, since
cc->order tells us the max free page order is smaller than it, otherwise the
allocation would happen already. I will add this condition in the next version.
>> + if (order <= MAX_ORDER) {
>> + low_pfn += (1UL << order) - 1;
>> + nr_scanned += (1UL << order) - 1;
>> + }
>> + goto isolate_fail;
>> }
>> - goto isolate_fail;
>> }
>> /*
>> @@ -1214,17 +1220,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>> goto isolate_abort;
>> }
>> }
>> -
>> - /*
>> - * folio become large since the non-locked check,
>> - * and it's on LRU.
>> - */
>> - if (unlikely(folio_test_large(folio) && !cc->alloc_contig)) > - low_pfn += folio_nr_pages(folio) - 1;
>> - nr_scanned += folio_nr_pages(folio) - 1;
>> - folio_set_lru(folio);
>> - goto isolate_fail_put;
>> - }
>
> I do not think you can remove this validation, since previous validation is lockless. So under the lock, we need re-check if the compound page order is larger than pageblock_order or cc->order, that need fail to isolate.
This check should go away, but a new order check for large folios should be
added. Will add it. Thanks.
--
Best Regards,
Yan, Zi
Attachment:
signature.asc
Description: OpenPGP digital signature
