Re: [PATCH] mm/thp: Correctly differentiate between mapped THP and PMD migration entry

From: Aneesh Kumar K.V
Date: Mon Oct 15 2018 - 04:32:59 EST

On 10/12/18 1:32 PM, Anshuman Khandual wrote:

On 10/09/2018 06:48 PM, Will Deacon wrote:
On Tue, Oct 09, 2018 at 04:04:21PM +0300, Kirill A. Shutemov wrote:
On Tue, Oct 09, 2018 at 09:28:58AM +0530, Anshuman Khandual wrote:
A normal mapped THP page at PMD level should be correctly differentiated
from a PMD migration entry while walking the page table. A mapped THP would
additionally check positive for pmd_present() along with pmd_trans_huge()
as compared to a PMD migration entry. This just adds a new conditional test
differentiating the two while walking the page table.

Fixes: 616b8371539a6 ("mm: thp: enable thp migration in generic path")
Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
On X86, pmd_trans_huge() and is_pmd_migration_entry() are always mutually
exclusive which makes the current conditional block work for both mapped
and migration entries. This is not same with arm64 where pmd_trans_huge()
returns positive for both mapped and migration entries. Could some one
please explain why pmd_trans_huge() has to return false for migration
entries which just install swap bits and its still a PMD ?

I guess it's just a design choice. Any reason why arm64 cannot do the

Anshuman, would it work to:

#define pmd_trans_huge(pmd) (pmd_present(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT))
yeah this works but some how does not seem like the right thing to do
but can be the very last option.

There can be other code paths that makes that assumption. I ended up doing the below for pmd_trans_huge on ppc64.

* Only returns true for a THP. False for pmd migration entry.
* We also need to return true when we come across a pte that
* in between a thp split. While splitting THP, we mark the pmd
* invalid (pmdp_invalidate()) before we set it with pte page
* address. A pmd_trans_huge() check against a pmd entry during that time
* should return true.
* We should not call this on a hugetlb entry. We should check for HugeTLB
* entry using vma->vm_flags
* The page table walk rule is explained in Documentation/vm/transhuge.rst
static inline int pmd_trans_huge(pmd_t pmd)
if (!pmd_present(pmd))
return false;

if (radix_enabled())
return radix__pmd_trans_huge(pmd);
return hash__pmd_trans_huge(pmd);