On 10/30/20 7:55 PM, Yang Shi wrote:
On Fri, Oct 30, 2020 at 11:39 AM Zi Yan <ziy@xxxxxxxxxx> wrote:
On 30 Oct 2020, at 14:33, Yang Shi wrote:
> On Fri, Oct 30, 2020 at 6:36 AM Michal Hocko <mhocko@xxxxxxxx> wrote:
>>
>> On Fri 30-10-20 08:20:50, Zi Yan wrote:
>>> On 30 Oct 2020, at 5:43, Michal Hocko wrote:
>>>
>>>> [Cc Vlastimil]
>>>>
>>>> On Thu 29-10-20 16:04:35, Zi Yan wrote:
>>>>> From: Zi Yan <ziy@xxxxxxxxxx>
>>>>>
>>>>> In isolate_migratepages_block, when cc->alloc_contig is true, we are
>>>>> able to isolate compound pages, nr_migratepages and nr_isolated did not
>>>>> count compound pages correctly, causing us to isolate more pages than we
>>>>> thought. Use thp_nr_pages to count pages. Otherwise, we might be trapped
>>>>> in too_many_isolated while loop, since the actual isolated pages can go
>>>>> up to COMPACT_CLUSTER_MAX*512=16384, where COMPACT_CLUSTER_MAX is 32,
>>>>> since we stop isolation after cc->nr_migratepages reaches to
>>>>> COMPACT_CLUSTER_MAX.
>>>>>
>>>>> In addition, after we fix the issue above, cc->nr_migratepages could
>>>>> never be equal to COMPACT_CLUSTER_MAX if compound pages are isolated,
>>>>> thus page isolation could not stop as we intended. Change the isolation
>>>>> stop condition to >=.
>>>>>
>>>>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>>>>> ---
>>>>> mm/compaction.c | 8 ++++----
>>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>>>> index ee1f8439369e..0683a4999581 100644
>>>>> --- a/mm/compaction.c
>>>>> +++ b/mm/compaction.c
>>>>> @@ -1012,8 +1012,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>>>>>
>>>>> isolate_success:
>>>>> list_add(&page->lru, &cc->migratepages);
>>>>> - cc->nr_migratepages++;
>>>>> - nr_isolated++;
>>>>> + cc->nr_migratepages += thp_nr_pages(page);
>>>>> + nr_isolated += thp_nr_pages(page);
>>>>
>>>> Does thp_nr_pages work for __PageMovable pages?
>>>
>>> Yes. It is the same as compound_nr() but compiled
>>> to 1 when THP is not enabled.
>>
>> I am sorry but I do not follow. First of all the implementation of the
>> two is different and also I was asking about __PageMovable which should
>> never be THP IIRC. Can they be compound though?
>
> I have the same question, can they be compound? If they can be
> compound, PageTransHuge() can't tell from THP and compound movable
> page, right?
Right. I have updated the patch and use compound_nr instead.
Thanks. Actually I'm wondering what kind of movable page could be
compound. Any real examples?
Looks like there's currently none. Compaction also wouldn't work properly with
movable pages with order>0 as the free page scanner looks for order-0 pages
only. But it won't hurt to use compound_nr() anyway.
—
Best Regards,
Yan Zi
