All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anshuman Khandual <anshuman.khandual@arm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org
Cc: mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 00/13] mm/debug_vm_pgtable fixes
Date: Fri, 21 Aug 2020 13:31:21 +0530	[thread overview]
Message-ID: <6927a5cf-4100-e43e-6aba-5d7bc0533276@arm.com> (raw)
In-Reply-To: <46cc2987-0d1e-f8e8-ecaf-2d246b33413e@linux.ibm.com>



On 08/21/2020 12:23 PM, Aneesh Kumar K.V wrote:
> On 8/21/20 9:03 AM, Anshuman Khandual wrote:
>>
>>
>> On 08/19/2020 07:15 PM, Aneesh Kumar K.V wrote:
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>>
>>>> This patch series includes fixes for debug_vm_pgtable test code so that
>>>> they follow page table updates rules correctly. The first two patches introduce
>>>> changes w.r.t ppc64. The patches are included in this series for completeness. We can
>>>> merge them via ppc64 tree if required.
>>>>
>>>> Hugetlb test is disabled on ppc64 because that needs larger change to satisfy
>>>> page table update rules.
>>>>
>>>> Changes from V1:
>>>> * Address review feedback
>>>> * drop test specific pfn_pte and pfn_pmd.
>>>> * Update ppc64 page table helper to add _PAGE_PTE
>>>>
>>>> Aneesh Kumar K.V (13):
>>>>    powerpc/mm: Add DEBUG_VM WARN for pmd_clear
>>>>    powerpc/mm: Move setting pte specific flags to pfn_pte
>>>>    mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
>>>>    mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge
>>>>      vmap support.
>>>>    mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with
>>>>      CONFIG_NUMA_BALANCING
>>>>    mm/debug_vm_pgtable/THP: Mark the pte entry huge before using
>>>>      set_pmd/pud_at
>>>>    mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an
>>>>      existing pte entry
>>>>    mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
>>>>    mm/debug_vm_pgtable/locks: Move non page table modifying test together
>>>>    mm/debug_vm_pgtable/locks: Take correct page table lock
>>>>    mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
>>>>    mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
>>>>    mm/debug_vm_pgtable: populate a pte entry before fetching it
>>>>
>>>>   arch/powerpc/include/asm/book3s/64/pgtable.h |  29 +++-
>>>>   arch/powerpc/include/asm/nohash/pgtable.h    |   5 -
>>>>   arch/powerpc/mm/book3s64/pgtable.c           |   2 +-
>>>>   arch/powerpc/mm/pgtable.c                    |   5 -
>>>>   include/linux/io.h                           |  12 ++
>>>>   mm/debug_vm_pgtable.c                        | 151 +++++++++++--------
>>>>   6 files changed, 127 insertions(+), 77 deletions(-)
>>>>
>>>
>>> BTW I picked a wrong branch when sending this. Attaching the diff
>>> against what I want to send.  pfn_pmd() no more updates _PAGE_PTE
>>> because that is handled by pmd_mkhuge().
>>>
>>> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
>>> index 3b4da7c63e28..e18ae50a275c 100644
>>> --- a/arch/powerpc/mm/book3s64/pgtable.c
>>> +++ b/arch/powerpc/mm/book3s64/pgtable.c
>>> @@ -141,7 +141,7 @@ pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>       unsigned long pmdv;
>>>         pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>> -    return __pmd(pmdv | pgprot_val(pgprot) | _PAGE_PTE);
>>> +    return pmd_set_protbits(__pmd(pmdv), pgprot);
>>>   }
>>>     pmd_t mk_pmd(struct page *page, pgprot_t pgprot)
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index 7d9f8e1d790f..cad61d22f33a 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -229,7 +229,7 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
>>>     static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
>>>   {
>>> -    pmd_t pmd = pfn_pmd(pfn, prot);
>>> +    pmd_t pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
>>>         if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
>>>           return;
>>>
>>
>> Cover letter does not mention which branch or tag this series applies on.
>> Just assumed it to be 5.9-rc1. Should the above changes be captured as a
>> pre-requisite patch ?
>>
>> Anyways, the series fails to be build on arm64.
>>
>> A) Without CONFIG_TRANSPARENT_HUGEPAGE
>>
>> mm/debug_vm_pgtable.c: In function ‘debug_vm_pgtable’:
>> mm/debug_vm_pgtable.c:1045:2: error: too many arguments to function ‘pmd_advanced_tests’
>>    pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
>>    ^~~~~~~~~~~~~~~~~~
>> mm/debug_vm_pgtable.c:366:20: note: declared here
>>   static void __init pmd_advanced_tests(struct mm_struct *mm,
>>                      ^~~~~~~~~~~~~~~~~~
>>
>> B) As mentioned previously, this should be solved by including <linux/io.h>
>>
>> mm/debug_vm_pgtable.c: In function ‘pmd_huge_tests’:
>> mm/debug_vm_pgtable.c:215:7: error: implicit declaration of function ‘arch_ioremap_pmd_supported’; did you mean ‘arch_disable_smp_support’? [-Werror=implicit-function-declaration]
>>    if (!arch_ioremap_pmd_supported())
>>         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Please make sure that the series builds on all enabled platforms i.e x86,
>> arm64, ppc32, ppc64, arc, s390 along with selectively enabling/disabling
>> all the features that make various #ifdefs in the test.
>>
> 
> I was hoping to get kernel test robot build report to verify that. But if you can help with that i have pushed a branch to github with reported build failure fixes.
> 
> https://github.com/kvaneesh/linux/tree/debug_vm_pgtable
> 
> I still haven't looked at the PMD_FOLDED feedback from Christophe because I am not sure i follow why we are checking for PMD folded there.

If this series does not build on existing enabled platforms, wondering
how effective the review could be, assuming that things would need to
change again to fix those build failures on various platforms. Getting
this to build here is essential, as not all page table constructs are
available across these platforms. Hence wondering, it might be better
if you could resend the series after fixing build issues.


WARNING: multiple messages have this Message-ID (diff)
From: Anshuman Khandual <anshuman.khandual@arm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 00/13] mm/debug_vm_pgtable fixes
Date: Fri, 21 Aug 2020 13:31:21 +0530	[thread overview]
Message-ID: <6927a5cf-4100-e43e-6aba-5d7bc0533276@arm.com> (raw)
In-Reply-To: <46cc2987-0d1e-f8e8-ecaf-2d246b33413e@linux.ibm.com>



On 08/21/2020 12:23 PM, Aneesh Kumar K.V wrote:
> On 8/21/20 9:03 AM, Anshuman Khandual wrote:
>>
>>
>> On 08/19/2020 07:15 PM, Aneesh Kumar K.V wrote:
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>>
>>>> This patch series includes fixes for debug_vm_pgtable test code so that
>>>> they follow page table updates rules correctly. The first two patches introduce
>>>> changes w.r.t ppc64. The patches are included in this series for completeness. We can
>>>> merge them via ppc64 tree if required.
>>>>
>>>> Hugetlb test is disabled on ppc64 because that needs larger change to satisfy
>>>> page table update rules.
>>>>
>>>> Changes from V1:
>>>> * Address review feedback
>>>> * drop test specific pfn_pte and pfn_pmd.
>>>> * Update ppc64 page table helper to add _PAGE_PTE
>>>>
>>>> Aneesh Kumar K.V (13):
>>>>    powerpc/mm: Add DEBUG_VM WARN for pmd_clear
>>>>    powerpc/mm: Move setting pte specific flags to pfn_pte
>>>>    mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
>>>>    mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge
>>>>      vmap support.
>>>>    mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with
>>>>      CONFIG_NUMA_BALANCING
>>>>    mm/debug_vm_pgtable/THP: Mark the pte entry huge before using
>>>>      set_pmd/pud_at
>>>>    mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an
>>>>      existing pte entry
>>>>    mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
>>>>    mm/debug_vm_pgtable/locks: Move non page table modifying test together
>>>>    mm/debug_vm_pgtable/locks: Take correct page table lock
>>>>    mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
>>>>    mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
>>>>    mm/debug_vm_pgtable: populate a pte entry before fetching it
>>>>
>>>>   arch/powerpc/include/asm/book3s/64/pgtable.h |  29 +++-
>>>>   arch/powerpc/include/asm/nohash/pgtable.h    |   5 -
>>>>   arch/powerpc/mm/book3s64/pgtable.c           |   2 +-
>>>>   arch/powerpc/mm/pgtable.c                    |   5 -
>>>>   include/linux/io.h                           |  12 ++
>>>>   mm/debug_vm_pgtable.c                        | 151 +++++++++++--------
>>>>   6 files changed, 127 insertions(+), 77 deletions(-)
>>>>
>>>
>>> BTW I picked a wrong branch when sending this. Attaching the diff
>>> against what I want to send.  pfn_pmd() no more updates _PAGE_PTE
>>> because that is handled by pmd_mkhuge().
>>>
>>> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
>>> index 3b4da7c63e28..e18ae50a275c 100644
>>> --- a/arch/powerpc/mm/book3s64/pgtable.c
>>> +++ b/arch/powerpc/mm/book3s64/pgtable.c
>>> @@ -141,7 +141,7 @@ pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>       unsigned long pmdv;
>>>         pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>> -    return __pmd(pmdv | pgprot_val(pgprot) | _PAGE_PTE);
>>> +    return pmd_set_protbits(__pmd(pmdv), pgprot);
>>>   }
>>>     pmd_t mk_pmd(struct page *page, pgprot_t pgprot)
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index 7d9f8e1d790f..cad61d22f33a 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -229,7 +229,7 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
>>>     static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
>>>   {
>>> -    pmd_t pmd = pfn_pmd(pfn, prot);
>>> +    pmd_t pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
>>>         if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
>>>           return;
>>>
>>
>> Cover letter does not mention which branch or tag this series applies on.
>> Just assumed it to be 5.9-rc1. Should the above changes be captured as a
>> pre-requisite patch ?
>>
>> Anyways, the series fails to be build on arm64.
>>
>> A) Without CONFIG_TRANSPARENT_HUGEPAGE
>>
>> mm/debug_vm_pgtable.c: In function ‘debug_vm_pgtable’:
>> mm/debug_vm_pgtable.c:1045:2: error: too many arguments to function ‘pmd_advanced_tests’
>>    pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
>>    ^~~~~~~~~~~~~~~~~~
>> mm/debug_vm_pgtable.c:366:20: note: declared here
>>   static void __init pmd_advanced_tests(struct mm_struct *mm,
>>                      ^~~~~~~~~~~~~~~~~~
>>
>> B) As mentioned previously, this should be solved by including <linux/io.h>
>>
>> mm/debug_vm_pgtable.c: In function ‘pmd_huge_tests’:
>> mm/debug_vm_pgtable.c:215:7: error: implicit declaration of function ‘arch_ioremap_pmd_supported’; did you mean ‘arch_disable_smp_support’? [-Werror=implicit-function-declaration]
>>    if (!arch_ioremap_pmd_supported())
>>         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Please make sure that the series builds on all enabled platforms i.e x86,
>> arm64, ppc32, ppc64, arc, s390 along with selectively enabling/disabling
>> all the features that make various #ifdefs in the test.
>>
> 
> I was hoping to get kernel test robot build report to verify that. But if you can help with that i have pushed a branch to github with reported build failure fixes.
> 
> https://github.com/kvaneesh/linux/tree/debug_vm_pgtable
> 
> I still haven't looked at the PMD_FOLDED feedback from Christophe because I am not sure i follow why we are checking for PMD folded there.

If this series does not build on existing enabled platforms, wondering
how effective the review could be, assuming that things would need to
change again to fix those build failures on various platforms. Getting
this to build here is essential, as not all page table constructs are
available across these platforms. Hence wondering, it might be better
if you could resend the series after fixing build issues.

  reply	other threads:[~2020-08-21  8:01 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19 13:00 [PATCH v2 00/13] mm/debug_vm_pgtable fixes Aneesh Kumar K.V
2020-08-19 13:00 ` Aneesh Kumar K.V
2020-08-19 13:00 ` [PATCH v2 01/13] powerpc/mm: Add DEBUG_VM WARN for pmd_clear Aneesh Kumar K.V
2020-08-19 13:00   ` Aneesh Kumar K.V
2020-08-19 13:00 ` [PATCH v2 02/13] powerpc/mm: Move setting pte specific flags to pfn_pte Aneesh Kumar K.V
2020-08-19 13:00   ` Aneesh Kumar K.V
2020-08-19 13:00 ` [PATCH v2 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value Aneesh Kumar K.V
2020-08-19 13:00   ` Aneesh Kumar K.V
2020-08-19 13:00 ` [PATCH v2 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support Aneesh Kumar K.V
2020-08-19 13:00   ` Aneesh Kumar K.V
2020-08-19 16:29   ` kernel test robot
2020-08-19 16:29     ` kernel test robot
2020-08-19 16:29     ` kernel test robot
2020-08-20  5:31     ` Aneesh Kumar K.V
2020-08-20  5:31       ` Aneesh Kumar K.V
2020-08-21  8:38   ` Anshuman Khandual
2020-08-21  8:38     ` Anshuman Khandual
2020-08-19 13:00 ` [PATCH v2 05/13] mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with CONFIG_NUMA_BALANCING Aneesh Kumar K.V
2020-08-19 13:00   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 06/13] mm/debug_vm_pgtable/THP: Mark the pte entry huge before using set_pmd/pud_at Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 07/13] mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an existing pte entry Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-20 14:32   ` Christophe Leroy
2020-08-21  7:14     ` Aneesh Kumar K.V
2020-08-21  8:20       ` Anshuman Khandual
2020-08-19 13:01 ` [PATCH v2 08/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 09/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 10/13] mm/debug_vm_pgtable/locks: Take correct page table lock Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-21  8:03   ` Anshuman Khandual
2020-08-21  8:03     ` Anshuman Khandual
2020-08-21  8:08     ` Aneesh Kumar K.V
2020-08-21  8:08       ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 11/13] mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64 Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:01 ` [PATCH v2 13/13] mm/debug_vm_pgtable: populate a pte entry before fetching it Aneesh Kumar K.V
2020-08-19 13:01   ` Aneesh Kumar K.V
2020-08-19 13:45 ` [PATCH v2 00/13] mm/debug_vm_pgtable fixes Aneesh Kumar K.V
2020-08-19 13:45   ` Aneesh Kumar K.V
2020-08-21  3:33   ` Anshuman Khandual
2020-08-21  3:33     ` Anshuman Khandual
2020-08-21  4:20     ` Anshuman Khandual
2020-08-21  4:20       ` Anshuman Khandual
2020-08-21  6:53     ` Aneesh Kumar K.V
2020-08-21  6:53       ` Aneesh Kumar K.V
2020-08-21  8:01       ` Anshuman Khandual [this message]
2020-08-21  8:01         ` Anshuman Khandual
2020-08-21  8:10         ` Aneesh Kumar K.V
2020-08-21  8:10           ` Aneesh Kumar K.V
2020-08-21  8:50           ` Aneesh Kumar K.V
2020-08-21  8:50             ` Aneesh Kumar K.V
2020-08-21  9:09             ` Anshuman Khandual
2020-08-21  9:09               ` Anshuman Khandual
2020-08-21  8:51 ` Anshuman Khandual
2020-08-21  8:51   ` Anshuman Khandual
2020-08-21  8:51   ` Anshuman Khandual
2020-08-21  8:51   ` Anshuman Khandual
2020-08-21  8:51   ` Anshuman Khandual
2020-09-01  8:03   ` Christophe Leroy
2020-09-01  8:03     ` Christophe Leroy
2020-09-01  8:03     ` Christophe Leroy
2020-09-01  8:03     ` Christophe Leroy
2020-09-01  8:03     ` Christophe Leroy
2020-09-01  9:11     ` Anshuman Khandual
2020-09-01  9:11       ` Anshuman Khandual
2020-09-01  9:11       ` Anshuman Khandual
2020-09-01  9:11       ` Anshuman Khandual
2020-09-01  9:11       ` Anshuman Khandual

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6927a5cf-4100-e43e-6aba-5d7bc0533276@arm.com \
    --to=anshuman.khandual@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.