Re: [PATCH v12 5/6] khugepaged: enable collapse pmd for pte-mapped THP

[Song Liu]

> On Aug 12, 2019, at 7:40 AM, Kirill A. Shutemov <kirill@xxxxxxxxxxxxx> wrote:
> 
> On Mon, Aug 12, 2019 at 03:22:58PM +0200, Oleg Nesterov wrote:
>> On 08/12, Kirill A. Shutemov wrote:
>>> 
>>> On Fri, Aug 09, 2019 at 06:01:18PM +0000, Song Liu wrote:
>>>> +		if (pte_none(*pte) || !pte_present(*pte))
>>>> +			continue;
>>> 
>>> You don't need to check both. Present is never none.
>> 
>> Agreed.
>> 
>> Kirill, while you are here, shouldn't retract_page_tables() check
>> vma->anon_vma (and probably do mm_find_pmd) under vm_mm->mmap_sem?
>> 
>> Can't it race with, say, do_cow_fault?
> 
> vma->anon_vma can race, but it doesn't matter. False-negative is fine.
> It's attempt to avoid taking mmap_sem where it can be not productive.
> 
> mm_find_pmd() cannot race with do_cow_fault() since the page is locked.
> __do_fault() has to return locked page before we touch page tables.
> It is somewhat subtle, but I wanted to avoid taking mmap_sem where it is
> possible.
> 
> -- 
> Kirill A. Shutemov

Updated version attached. 


Besides feedbacks from Oleg and Kirill, I also revise the locking in 
collapse_pte_mapped_thp(): use pte_offset_map_lock() for the two loops 
to cover highmem. zap_pte_range() has similar use of the lock. 

This change is suggested by Johannes. 

Thanks,
Song

================ 8< ======================