2021-04-01 02:25:07

by Liu Shixin

Subject: [PATCH -next 1/3] mm/debug_vm_pgtable: Fix one comment mistake

The branch condition should be CONFIG_TRANSPARENT_HUGEPAGE instead of
CONFIG_ARCH_HAS_PTE_DEVMAP.

Signed-off-by: Shixin Liu <[email protected]>
---
mm/debug_vm_pgtable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 05efe98a9ac2..a5c71a94e804 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -755,12 +755,12 @@ static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd)));
WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd)));
}
-#else /* !CONFIG_ARCH_HAS_PTE_DEVMAP */
+#else /* !CONFIG_TRANSPARENT_HUGEPAGE */
static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot) { }
static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
{
}
-#endif /* CONFIG_ARCH_HAS_PTE_DEVMAP */
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */

static void __init pte_swap_tests(unsigned long pfn, pgprot_t prot)
{
--
2.25.1

2021-04-01 02:25:19

by Liu Shixin

Subject: [PATCH -next 3/3] mm/debug_vm_pgtable: Remove useless pfn_pmd()

The call to pfn_pmd() here is redundant.

Signed-off-by: Shixin Liu <[email protected]>
---
mm/debug_vm_pgtable.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index c379bbe42c2a..9f4c4a114229 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -196,7 +196,6 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,

pgtable_trans_huge_deposit(mm, pmdp, pgtable);

- pmd = pfn_pmd(pfn, prot);
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_set_wrprotect(mm, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
--
2.25.1

2021-04-01 02:25:30

by Liu Shixin

Subject: [PATCH -next 2/3] mm/debug_vm_pgtable: Move {pmd/pud}_huge_tests out of CONFIG_TRANSPARENT_HUGEPAGE

The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge is not depend on THP.
But now if we want to test these functions, we have to enable THP. So move
{pmd/pud}_huge_tests out of CONFIG_TRANSPARENT_HUGEPAGE.

Signed-off-by: Shixin Liu <[email protected]>
---
mm/debug_vm_pgtable.c | 91 +++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 52 deletions(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index a5c71a94e804..c379bbe42c2a 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -242,29 +242,6 @@ static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
WARN_ON(!pmd_leaf(pmd));
}

-#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
-static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
-{
- pmd_t pmd;
-
- if (!arch_vmap_pmd_supported(prot))
- return;
-
- pr_debug("Validating PMD huge\n");
- /*
- * X86 defined pmd_set_huge() verifies that the given
- * PMD is not a populated non-leaf entry.
- */
- WRITE_ONCE(*pmdp, __pmd(0));
- WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot));
- WARN_ON(!pmd_clear_huge(pmdp));
- pmd = READ_ONCE(*pmdp);
- WARN_ON(!pmd_none(pmd));
-}
-#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
-static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
-#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
-
static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
{
pmd_t pmd = pfn_pmd(pfn, prot);
@@ -379,30 +356,6 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
pud = pud_mkhuge(pud);
WARN_ON(!pud_leaf(pud));
}
-
-#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
-static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
-{
- pud_t pud;
-
- if (!arch_vmap_pud_supported(prot))
- return;
-
- pr_debug("Validating PUD huge\n");
- /*
- * X86 defined pud_set_huge() verifies that the given
- * PUD is not a populated non-leaf entry.
- */
- WRITE_ONCE(*pudp, __pud(0));
- WARN_ON(!pud_set_huge(pudp, __pfn_to_phys(pfn), prot));
- WARN_ON(!pud_clear_huge(pudp));
- pud = READ_ONCE(*pudp);
- WARN_ON(!pud_none(pud));
-}
-#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
-static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
-#endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
-
#else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { }
static void __init pud_advanced_tests(struct mm_struct *mm,
@@ -412,9 +365,6 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
{
}
static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
-static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
-{
-}
#endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
#else /* !CONFIG_TRANSPARENT_HUGEPAGE */
static void __init pmd_basic_tests(unsigned long pfn, int idx) { }
@@ -433,14 +383,51 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
}
static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot) { }
static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
+static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) { }
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
+#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
{
+ pmd_t pmd;
+
+ if (!arch_vmap_pmd_supported(prot))
+ return;
+
+ pr_debug("Validating PMD huge\n");
+ /*
+ * X86 defined pmd_set_huge() verifies that the given
+ * PMD is not a populated non-leaf entry.
+ */
+ WRITE_ONCE(*pmdp, __pmd(0));
+ WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot));
+ WARN_ON(!pmd_clear_huge(pmdp));
+ pmd = READ_ONCE(*pmdp);
+ WARN_ON(!pmd_none(pmd));
}
+
static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
{
+ pud_t pud;
+
+ if (!arch_vmap_pud_supported(prot))
+ return;
+
+ pr_debug("Validating PUD huge\n");
+ /*
+ * X86 defined pud_set_huge() verifies that the given
+ * PUD is not a populated non-leaf entry.
+ */
+ WRITE_ONCE(*pudp, __pud(0));
+ WARN_ON(!pud_set_huge(pudp, __pfn_to_phys(pfn), prot));
+ WARN_ON(!pud_clear_huge(pudp));
+ pud = READ_ONCE(*pudp);
+ WARN_ON(!pud_none(pud));
}
-static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) { }
-#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
+static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
+static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
+#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */

static void __init p4d_basic_tests(unsigned long pfn, pgprot_t prot)
{
--
2.25.1

2021-04-01 18:11:38

by Anshuman Khandual

Subject: Re: [PATCH -next 2/3] mm/debug_vm_pgtable: Move {pmd/pud}_huge_tests out of CONFIG_TRANSPARENT_HUGEPAGE

On 4/1/21 7:53 AM, Shixin Liu wrote:
> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge is not depend on THP.

s/is not depend/are not dependent/

> But now if we want to test these functions, we have to enable THP. So move
> {pmd/pud}_huge_tests out of CONFIG_TRANSPARENT_HUGEPAGE.

Please drop the second sentence here.

>
> Signed-off-by: Shixin Liu <[email protected]>
> ---
> mm/debug_vm_pgtable.c | 91 +++++++++++++++++++------------------------
> 1 file changed, 39 insertions(+), 52 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index a5c71a94e804..c379bbe42c2a 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -242,29 +242,6 @@ static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pmd_leaf(pmd));
> }
>
> -#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> -{
> - pmd_t pmd;
> -
> - if (!arch_vmap_pmd_supported(prot))
> - return;
> -
> - pr_debug("Validating PMD huge\n");
> - /*
> - * X86 defined pmd_set_huge() verifies that the given
> - * PMD is not a populated non-leaf entry.
> - */
> - WRITE_ONCE(*pmdp, __pmd(0));
> - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot));
> - WARN_ON(!pmd_clear_huge(pmdp));
> - pmd = READ_ONCE(*pmdp);
> - WARN_ON(!pmd_none(pmd));
> -}
> -#else /* CONFIG_HAVE_ARCH_HUGE_VMAP */
> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
> -#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
> -
> static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
> {
> pmd_t pmd = pfn_pmd(pfn, prot);
> @@ -379,30 +356,6 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
> pud = pud_mkhuge(pud);
> WARN_ON(!pud_leaf(pud));
> }
> -
> -#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> -static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> -{
> - pud_t pud;
> -
> - if (!arch_vmap_pud_supported(prot))
> - return;
> -
> - pr_debug("Validating PUD huge\n");
> - /*
> - * X86 defined pud_set_huge() verifies that the given
> - * PUD is not a populated non-leaf entry.
> - */
> - WRITE_ONCE(*pudp, __pud(0));
> - WARN_ON(!pud_set_huge(pudp, __pfn_to_phys(pfn), prot));
> - WARN_ON(!pud_clear_huge(pudp));
> - pud = READ_ONCE(*pudp);
> - WARN_ON(!pud_none(pud));
> -}
> -#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> -static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
> -#endif /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> -
> #else /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx) { }
> static void __init pud_advanced_tests(struct mm_struct *mm,
> @@ -412,9 +365,6 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> {
> }
> static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
> -static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> -{
> -}
> #endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */
> static void __init pmd_basic_tests(unsigned long pfn, int idx) { }
> @@ -433,14 +383,51 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> }
> static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot) { }
> +static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) { }
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +
> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> {
> + pmd_t pmd;
> +
> + if (!arch_vmap_pmd_supported(prot))
> + return;
> +
> + pr_debug("Validating PMD huge\n");
> + /*
> + * X86 defined pmd_set_huge() verifies that the given
> + * PMD is not a populated non-leaf entry.
> + */
> + WRITE_ONCE(*pmdp, __pmd(0));
> + WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot));
> + WARN_ON(!pmd_clear_huge(pmdp));
> + pmd = READ_ONCE(*pmdp);
> + WARN_ON(!pmd_none(pmd));
> }
> +
> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> {
> + pud_t pud;
> +
> + if (!arch_vmap_pud_supported(prot))
> + return;
> +
> + pr_debug("Validating PUD huge\n");
> + /*
> + * X86 defined pud_set_huge() verifies that the given
> + * PUD is not a populated non-leaf entry.
> + */
> + WRITE_ONCE(*pudp, __pud(0));
> + WARN_ON(!pud_set_huge(pudp, __pfn_to_phys(pfn), prot));
> + WARN_ON(!pud_clear_huge(pudp));
> + pud = READ_ONCE(*pudp);
> + WARN_ON(!pud_none(pud));
> }
> -static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot) { }
> -#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +#else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */
> +static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { }
> +static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { }
> +#endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */
>
> static void __init p4d_basic_tests(unsigned long pfn, pgprot_t prot)
> {
>

This overall does make sense. Will further look into this but later on.

2021-04-01 18:37:09

by Anshuman Khandual

Subject: Re: [PATCH -next 1/3] mm/debug_vm_pgtable: Fix one comment mistake

On 4/1/21 7:53 AM, Shixin Liu wrote:
> The branch condition should be CONFIG_TRANSPARENT_HUGEPAGE instead of
> CONFIG_ARCH_HAS_PTE_DEVMAP.
>
> Signed-off-by: Shixin Liu <[email protected]>
> ---
> mm/debug_vm_pgtable.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 05efe98a9ac2..a5c71a94e804 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -755,12 +755,12 @@ static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
> WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd)));
> WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd)));
> }
> -#else /* !CONFIG_ARCH_HAS_PTE_DEVMAP */
> +#else /* !CONFIG_TRANSPARENT_HUGEPAGE */
> static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot) { }
> static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
> {
> }
> -#endif /* CONFIG_ARCH_HAS_PTE_DEVMAP */
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> static void __init pte_swap_tests(unsigned long pfn, pgprot_t prot)
> {
>

LGTM, thanks for catching. But does not need a patch of it's
own, instead should be folded with other potential clean ups.

2021-04-01 18:44:22

by Anshuman Khandual

Subject: Re: [PATCH -next 3/3] mm/debug_vm_pgtable: Remove useless pfn_pmd()

On 4/1/21 7:53 AM, Shixin Liu wrote:
> The call to pfn_pmd() here is redundant.
>
> Signed-off-by: Shixin Liu <[email protected]>
> ---
> mm/debug_vm_pgtable.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index c379bbe42c2a..9f4c4a114229 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -196,7 +196,6 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
>
> pgtable_trans_huge_deposit(mm, pmdp, pgtable);
>
> - pmd = pfn_pmd(pfn, prot);
> set_pmd_at(mm, vaddr, pmdp, pmd);
> pmdp_set_wrprotect(mm, vaddr, pmdp);
> pmd = READ_ONCE(*pmdp);
>

Instead drop the first pfn_pmd(), as that pmd would not be required
when THP is not enabled. Also, please fold this with the first patch.