[RFC PATCH] powerpc/64s: __find_linux_pte synchronization vs pmdp_invalidate

[RFC PATCH] powerpc/64s: __find_linux_pte synchronization vs pmdp_invalidate

[Nicholas Piggin]

The pmd_none check does not catch hugepage collapse, nor does the
pmd_present check in pmd_trans_huge, because hugepage collapse sets
!_PAGE_PRESENT && _PAGE_INVALID (which results in !pmd_none and
pmd_present).

Aneesh noticed we might need this check as well.

---
 arch/powerpc/mm/pgtable.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index db4a6253df92..7a702d21400a 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -372,13 +372,20 @@ pte_t *__find_linux_pte(pgd_t *pgdir, unsigned long ea,
 	pdshift = PMD_SHIFT;
 	pmdp = pmd_offset(&pud, ea);
 	pmd  = READ_ONCE(*pmdp);
-	/*
-	 * A hugepage collapse is captured by pmd_none, because
-	 * it mark the pmd none and do a hpte invalidate.
-	 */
+
 	if (pmd_none(pmd))
 		return NULL;
 
+#ifdef CONFIG_PPC_BOOK3S_64
+	if (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_INVALID) == _PAGE_INVALID) {
+		/*
+		 * A hugepage collapse is captured by this condition, see
+		 * pmdp_invalidate.
+		 */
+		return NULL;
+	}
+#endif
+
 	if (pmd_trans_huge(pmd) || pmd_devmap(pmd)) {
 		if (is_thp)
 			*is_thp = true;
-- 
2.20.1

Re: [RFC PATCH] powerpc/64s: __find_linux_pte synchronization vs pmdp_invalidate

[Christophe Leroy]

Le 03/06/2019 à 08:44, Nicholas Piggin a écrit :
> The pmd_none check does not catch hugepage collapse, nor does the
> pmd_present check in pmd_trans_huge, because hugepage collapse sets
> !_PAGE_PRESENT && _PAGE_INVALID (which results in !pmd_none and
> pmd_present).
> 
> Aneesh noticed we might need this check as well.
> 
> ---
>   arch/powerpc/mm/pgtable.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index db4a6253df92..7a702d21400a 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -372,13 +372,20 @@ pte_t *__find_linux_pte(pgd_t *pgdir, unsigned long ea,
>   	pdshift = PMD_SHIFT;
>   	pmdp = pmd_offset(&pud, ea);
>   	pmd  = READ_ONCE(*pmdp);
> -	/*
> -	 * A hugepage collapse is captured by pmd_none, because
> -	 * it mark the pmd none and do a hpte invalidate.
> -	 */
> +
>   	if (pmd_none(pmd))
>   		return NULL;
>   
> +#ifdef CONFIG_PPC_BOOK3S_64

I can't see anything that would build fail on other subarches. Wouldn't 
be better to use

if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && (pmd_val(pmd) & 
(_PAGE_PRESENT|_PAGE_INVALID) == _PAGE_INVALID))


> +	if (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_INVALID) == _PAGE_INVALID) {

Maybe using pmd_raw() instead as in all the book3s64 helpers ?

Christophe


> +		/*
> +		 * A hugepage collapse is captured by this condition, see
> +		 * pmdp_invalidate.
> +		 */
> +		return NULL;
> +	}
> +#endif
> +
>   	if (pmd_trans_huge(pmd) || pmd_devmap(pmd)) {
>   		if (is_thp)
>   			*is_thp = true;
>