On 09/03/2020 10:26 PM, Catalin Marinas wrote: > On Mon, Aug 17, 2020 at 02:49:43PM +0530, Anshuman Khandual wrote: >> pmd_present() and pmd_trans_huge() are expected to behave in the following >> manner during various phases of a given PMD. It is derived from a previous >> detailed discussion on this topic [1] and present THP documentation [2]. >> >> pmd_present(pmd): >> >> - Returns true if pmd refers to system RAM with a valid pmd_page(pmd) >> - Returns false if pmd does not refer to system RAM - Invalid pmd_page(pmd) > > The second bullet doesn't make much sense. If you have a pmd mapping of > some I/O memory, pmd_present() still returns true (as does > pte_present()). Derived this from an earlier discussion (https://lkml.org/lkml/2018/10/17/231) but current representation here might not be accurate. Would this be any better ? pmd_present(pmd): - Returns true if pmd refers to system RAM with a valid pmd_page(pmd) - Returns false if pmd refers to a migration or swap entry > >> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h >> index 4d867c6446c4..28792fdd9627 100644 >> --- a/arch/arm64/include/asm/pgtable-prot.h >> +++ b/arch/arm64/include/asm/pgtable-prot.h >> @@ -19,6 +19,13 @@ >> #define PTE_DEVMAP (_AT(pteval_t, 1) << 57) >> #define PTE_PROT_NONE (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */ >> >> +/* >> + * This help indicate that the entry is present i.e pmd_page() > > Nit: add another . after i.e Will fix. > >> + * still points to a valid huge page in memory even if the pmd >> + * has been invalidated. >> + */ >> +#define PMD_PRESENT_INVALID (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */ >> + >> #ifndef __ASSEMBLY__ >> >> #include <asm/cpufeature.h> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index d5d3fbe73953..7aa69cace784 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -145,6 +145,18 @@ static inline pte_t set_pte_bit(pte_t pte, pgprot_t prot) >> return pte; >> } >> >> +static inline pmd_t clr_pmd_bit(pmd_t pmd, pgprot_t prot) >> +{ >> + pmd_val(pmd) &= ~pgprot_val(prot); >> + return pmd; >> +} > > Could you use clear_pmd_bit (instead of clr) for consistency with > clear_pte_bit()? Sure, will do. > > It would be good if the mm folk can do a sanity check on the assumptions > about pmd_present/pmdp_invalidate/pmd_trans_huge. > > The patch looks fine to me otherwise, feel free to add: > > Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx> >
