2021-04-06 13:14:38

by Liu Shixin

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

v1->v2:
Modified the commit message.

The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.

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 05efe98a9ac2..d3cf178621d9 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-06 15:23:29

by Liu Shixin

[permalink] [raw]
Subject: [PATCH -next v2 2/2] mm/debug_vm_pgtable: Remove redundant pfn_{pmd/pte}() and fix one comment mistake

v1->v2:
Remove redundant pfn_pte() and fold two patch to one.

Remove redundant pfn_{pmd/pte}() and fix one comment mistake.

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

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index d3cf178621d9..e2f35db8ba69 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -91,7 +91,7 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
unsigned long pfn, unsigned long vaddr,
pgprot_t prot)
{
- pte_t pte = pfn_pte(pfn, prot);
+ pte_t pte;

/*
* Architectures optimize set_pte_at by avoiding TLB flush.
@@ -185,7 +185,7 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
unsigned long pfn, unsigned long vaddr,
pgprot_t prot, pgtable_t pgtable)
{
- pmd_t pmd = pfn_pmd(pfn, prot);
+ pmd_t pmd;

if (!has_transparent_hugepage())
return;
@@ -300,7 +300,7 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
unsigned long pfn, unsigned long vaddr,
pgprot_t prot)
{
- pud_t pud = pfn_pud(pfn, prot);
+ pud_t pud;

if (!has_transparent_hugepage())
return;
@@ -309,6 +309,7 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
/* Align the address wrt HPAGE_PUD_SIZE */
vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;

+ pud = pfn_pud(pfn, prot);
set_pud_at(mm, vaddr, pudp, pud);
pudp_set_wrprotect(mm, vaddr, pudp);
pud = READ_ONCE(*pudp);
@@ -742,12 +743,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-09 03:41:54

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH -next v2 2/2] mm/debug_vm_pgtable: Remove redundant pfn_{pmd/pte}() and fix one comment mistake


On 4/6/21 10:19 AM, Shixin Liu wrote:
> v1->v2:
> Remove redundant pfn_pte() and fold two patch to one.

Change log should always be after the '---' below the SOB statement for git
am to ignore them. Please avoid adding them in the commit messages.

>
> Remove redundant pfn_{pmd/pte}() and fix one comment mistake.
>
> Signed-off-by: Shixin Liu <[email protected]>
> ---
> mm/debug_vm_pgtable.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index d3cf178621d9..e2f35db8ba69 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -91,7 +91,7 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
> unsigned long pfn, unsigned long vaddr,
> pgprot_t prot)
> {
> - pte_t pte = pfn_pte(pfn, prot);
> + pte_t pte;
>

Right.

> /*
> * Architectures optimize set_pte_at by avoiding TLB flush.
> @@ -185,7 +185,7 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
> unsigned long pfn, unsigned long vaddr,
> pgprot_t prot, pgtable_t pgtable)
> {
> - pmd_t pmd = pfn_pmd(pfn, prot);
> + pmd_t pmd;


Right.

>
> if (!has_transparent_hugepage())
> return;
> @@ -300,7 +300,7 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> unsigned long pfn, unsigned long vaddr,
> pgprot_t prot)
> {
> - pud_t pud = pfn_pud(pfn, prot);
> + pud_t pud;
>
> if (!has_transparent_hugepage())
> return;
> @@ -309,6 +309,7 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
> /* Align the address wrt HPAGE_PUD_SIZE */
> vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;
>
> + pud = pfn_pud(pfn, prot);

Is this change intended to make pud_advanced_tests() similar other
advanced tests ? Please update the commit message as well.

> set_pud_at(mm, vaddr, pudp, pud);
> pudp_set_wrprotect(mm, vaddr, pudp);
> pud = READ_ONCE(*pudp);
> @@ -742,12 +743,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)
> {
>
With changes to the commit message as suggested earlier.

Reviewed-by: Anshuman Khandual <[email protected]>

2021-04-09 04:07:02

by Anshuman Khandual

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


On 4/6/21 10:18 AM, Shixin Liu wrote:
> v1->v2:
> Modified the commit message.

Please avoid change log in the commit message, it should be after '---'
below the SOB statement.

>
> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.

typo ^^^^^ s/ars/are

Also there is a checkpatch.pl warning.

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10:
The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.

total: 0 errors, 1 warnings, 121 lines checked

As I had mentioned in the earlier version, the commit message should be some
thing like ..

----
The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge are not dependent
on THP. Hence 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 05efe98a9ac2..d3cf178621d9 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)
> {
>

With changes to the commit message as suggested earlier.

Reviewed-by: Anshuman Khandual <[email protected]>

2021-04-19 03:30:39

by Anshuman Khandual

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



On 4/9/21 9:35 AM, Anshuman Khandual wrote:
>
> On 4/6/21 10:18 AM, Shixin Liu wrote:
>> v1->v2:
>> Modified the commit message.
>
> Please avoid change log in the commit message, it should be after '---'
> below the SOB statement.
>
>>
>> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.
>
> typo ^^^^^ s/ars/are
>
> Also there is a checkpatch.pl warning.
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #10:
> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.
>
> total: 0 errors, 1 warnings, 121 lines checked
>
> As I had mentioned in the earlier version, the commit message should be some
> thing like ..
>
> ----
> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge are not dependent
> on THP. Hence 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 05efe98a9ac2..d3cf178621d9 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)
>> {
>>
>
> With changes to the commit message as suggested earlier.
>
> Reviewed-by: Anshuman Khandual <[email protected]>
>

Hello Shixin,

Wondering if you are planning to respin the series ?

- Anshuman

2021-04-19 06:37:04

by Liu Shixin

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

Thanks for your advice. I will fix these patches and resend them as soon as possilble.


On 2021/4/19 11:30, Anshuman Khandual wrote:
>
> On 4/9/21 9:35 AM, Anshuman Khandual wrote:
>> On 4/6/21 10:18 AM, Shixin Liu wrote:
>>> v1->v2:
>>> Modified the commit message.
>> Please avoid change log in the commit message, it should be after '---'
>> below the SOB statement.
>>
>>> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.
>> typo ^^^^^ s/ars/are
>>
>> Also there is a checkpatch.pl warning.
>>
>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
>> #10:
>> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge ars not dependent on THP.
>>
>> total: 0 errors, 1 warnings, 121 lines checked
>>
>> As I had mentioned in the earlier version, the commit message should be some
>> thing like ..
>>
>> ----
>> The functions {pmd/pud}_set_huge and {pmd/pud}_clear_huge are not dependent
>> on THP. Hence 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 05efe98a9ac2..d3cf178621d9 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)
>>> {
>>>
>> With changes to the commit message as suggested earlier.
>>
>> Reviewed-by: Anshuman Khandual <[email protected]>
>>
> Hello Shixin,
>
> Wondering if you are planning to respin the series ?
>
> - Anshuman
> .
>