Hi Oleg, > On Aug 12, 2019, at 2:04 PM, Song Liu <songliubraving@xxxxxx> wrote: > > > >> 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. > Do you have further comments for the version below? If not, could you please reply with your Acked-by or Reviewed-by? Thanks, Song > > ================ 8< ====================== > From 3d931bc4780abb6109fe478a4b1a0004ce81efe1 Mon Sep 17 00:00:00 2001 > From: Song Liu <songliubraving@xxxxxx> > Date: Sun, 28 Jul 2019 03:43:48 -0700 > Subject: [PATCH 5/6] khugepaged: enable collapse pmd for pte-mapped THP > [...]