On 2021/3/24 9:16, Yang Shi wrote:
> On Tue, Mar 23, 2021 at 10:17 AM Yang Shi <shy828301@xxxxxxxxx> wrote:
>>
>> On Tue, Mar 23, 2021 at 6:55 AM Miaohe Lin <linmiaohe@xxxxxxxxxx> wrote:
>>>
>>> Since commit c77c5cbafe54 ("mm: migrate: skip shared exec THP for NUMA
>>> balancing"), the NUMA balancing would skip shared exec transhuge page.
>>> But this enhancement is not suitable for transhuge page. Because it's
>>> required that page_mapcount() must be 1 due to no migration pte dance
>>> is done here. On the other hand, the shared exec transhuge page will
>>> leave the migrate_misplaced_page() with pte entry untouched and page
>>> locked. Thus pagefault for NUMA will be triggered again and deadlock
>>> occurs when we start waiting for the page lock held by ourselves.
>>
>> Thanks for catching this. By relooking the code I think the other
>> important reason for removing this is
>> migrate_misplaced_transhuge_page() actually can't see shared exec file
>> THP at all since page_lock_anon_vma_read() is called before and if
>> page is not anonymous page it will just restore the PMD without
>> migrating anything.
>>
>> The pages for private mapped file vma may be anonymous pages due to
>> COW but they can't be THP so it won't trigger THP numa fault at all. I
>> think this is why no bug was reported. I overlooked this in the first
>> place.
>>
>> Your fix is correct, and please add the above justification to your commit log.
>
> BTW, I think you can just undo or revert commit c77c5cbafe54 ("mm:
> migrate: skip shared exec THP for NUMA balancing").
>
Yep, we can revert this commit. I thought it handle the shared exec base page too.
Will do it and with the above justification to the commit log.
Many Thanks!
> Thanks,
> Yang
>
>>
>> Reviewed-by: Yang Shi <shy828301@xxxxxxxxx>
>>
>>>
>>> Fixes: c77c5cbafe54 ("mm: migrate: skip shared exec THP for NUMA balancing")
>>> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>>> ---
>>> mm/migrate.c | 4 ----
>>> 1 file changed, 4 deletions(-)
>>>
>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>> index 5357a8527ca2..68bfa1625898 100644
>>> --- a/mm/migrate.c
>>> +++ b/mm/migrate.c
>>> @@ -2192,9 +2192,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
>>> int page_lru = page_is_file_lru(page);
>>> unsigned long start = address & HPAGE_PMD_MASK;
>>>
>>> - if (is_shared_exec_page(vma, page))
>>> - goto out;
>>> -
>>> new_page = alloc_pages_node(node,
>>> (GFP_TRANSHUGE_LIGHT | __GFP_THISNODE),
>>> HPAGE_PMD_ORDER);
>>> @@ -2306,7 +2303,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
>>>
>>> out_unlock:
>>> unlock_page(page);
>>> -out:
>>> put_page(page);
>>> return 0;
>>> }
>>> --
>>> 2.19.1
>>>
> .
>
