With the hash page table, the kernel should not use pmd_clear for clearing
huge pte entries. Add a DEBUG_VM WARN to catch the wrong usage.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
random value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
set_pte_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pte_at() and hence expect it to be used to set locations
that are not a valid PTE.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Saved write support was added to track the write bit of a pte after marking the
pte protnone. This was done so that AUTONUMA can convert a write pte to protnone
and still track the old write bit. When converting it back we set the pte write
bit correctly thereby avoiding a write fault again. Hence enable the test only
when CONFIG_NUMA_BALANCING is enabled.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 7 +++++++
1 file changed, 7 insertions(+)
set_pmd_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pmd_at() and hence expect it to be used to set locations
that are not a valid PTE.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
set_pud_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pud_at() and hence expect it to be used to set locations
that are not a valid PTE.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
This will help in adding proper locks in a later patch
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 53 +++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 24 deletions(-)
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Saved write support was added to track the write bit of a pte after marking the
pte protnone. This was done so that AUTONUMA can convert a write pte to protnone
and still track the old write bit. When converting it back we set the pte write
bit correctly thereby avoiding a write fault again.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
The tests do expect _PAGE_PTE bit set by different page table accessors.
This is not true for the kernel. Within the kernel, _PAGE_PTE bits are
usually set by set_pte_at(). To make the below tests work correctly add test
specific pfn_pte/pmd helpers that set _PAGE_PTE bit.
pte_t pte = pfn_pte(pfn, prot);
WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 65 +++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 24 deletions(-)
ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
random value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting
that bit in
random value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
* pxx_clear() because of how dynamic page table folding works on
s390. So
* while loading up the entries do not change the lower 4 bits. It
does not
* have affect any other platform.
+ *
+ * Also avoid the 62nd bit on ppc64 that is used to mark a pte entry.
*/
#define S390_MASK_BITS 4
-#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
+#define PPC_MASK_BITS 2
+#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1 - PPC_MASK_BITS,
S390_MASK_BITS)
Do you mean:
#define RANDOM_ORVALUE GENMASK(BITS_PER_LONG - 1, PPC_MASK_BITS |
S390_MASK_BITS)
IIUC GENMASK(hi, low) generate a mask from hi to low bits. Since i want
to avoid bit 62, I am forcing it to generate bits from (61, 4)
-aneesh
With the hash page table, the kernel should not use pmd_clear for clearing
huge pte entries. Add a DEBUG_VM WARN to catch the wrong usage.
Signed-off-by: Aneesh Kumar K.V <redacted>
This particular change is very much powerpc specific. Hence please drop it from
the series which otherwise changes the page table test. Also, this series which
is not a RFC, still lacks a proper cover letter with diff stats, tree/tag on
which this applies, summary about the proposal etc. All those information will
be helpful in reviewing this series better. For now, assuming that this applies
cleanly on current master branch. But again, please do include a cover letter
in the next version.
ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
random value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
With this change, RANDOM_ORVALUE will be (0x3ffffffffffffff0) which discards
both bit 63 and 62. If only bit 62 is to be avoided for ppc64 the mask should
be (0xbffffffffffffff0) instead. The following change on this patch should do
the trick.
ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
random value.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
With this change, RANDOM_ORVALUE will be (0x3ffffffffffffff0) which discards
both bit 63 and 62. If only bit 62 is to be avoided for ppc64 the mask should
be (0xbffffffffffffff0) instead. The following change on this patch should do
the trick.
With the hash page table, the kernel should not use pmd_clear for clearing
huge pte entries. Add a DEBUG_VM WARN to catch the wrong usage.
Signed-off-by: Aneesh Kumar K.V <redacted>
This particular change is very much powerpc specific. Hence please drop it from
the series which otherwise changes the page table test. Also, this series which
is not a RFC, still lacks a proper cover letter with diff stats, tree/tag on
which this applies, summary about the proposal etc. All those information will
be helpful in reviewing this series better. For now, assuming that this applies
cleanly on current master branch. But again, please do include a cover letter
in the next version.
The patch series include all sort of fixes. There is no special theme
for the series. So all that the cover letter would have is "fixes to
make debug_vm_pgtable work on ppc64"
I tried to keep each patch simpler explaining why the current code is
wrong.
set_pte_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pte_at() and hence expect it to be used to set locations
that are not a valid PTE.
Even though set_pte_at() is not really a arch page table helper and
very much arch specific, I just wonder why this deviation on ppc64
as compared to other platforms. Detecting such semantics variation
across platforms is an objective of this test.
As small nit.
Please follow the existing subject format for all patches in here.
It will improve readability and also help understand these changes
better, later on.
mm/debug_vm_pgtable: <Specify changes to an individual test>
This makes sense. But could you please fold this code stanza with
the previous one in order to imply that 'ptep' did have some valid
entry before being cleared and checked against pte_none().
+ /*
+ * We should clear pte before we do set_pte_at
+ */
+ pte = ptep_get_and_clear(mm, vaddr, ptep);
pte = pte_mkyoung(pte);
set_pte_at(mm, vaddr, ptep, pte);
ptep_test_and_clear_young(vma, vaddr, ptep);
The comment above should also explain details that are mentioned
in the commit message i.e how platforms such as ppc64 expects a
clear pte entry for set_pte_at() to work.
set_pte_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pte_at() and hence expect it to be used to set locations
that are not a valid PTE.
Even though set_pte_at() is not really a arch page table helper and
very much arch specific, I just wonder why this deviation on ppc64
as compared to other platforms. Detecting such semantics variation
across platforms is an objective of this test.
Not sure what you mean by set_pte_at is not a page table helper. Generic
kernel use that helper to set a pte entry.
Now w.r.t ppc64 behavior this was discussed multiple times. I guess
Linux kernel always used set_pte_at on a none pte entry. We had some
exceptions in the recent past. But all fixed when noticed.
383321ab8578dfe3bbcc0bc5604c0f8ae08a5c98
mm/hugetlb/migration: use set_huge_pte_at instead of set_pte_at
cee216a696b2004017a5ecb583366093d90b1568
mm/autonuma: don't use set_pte_at when updating protnone ptes
56eecdb912b536a4fa97fb5bfe5a940a54d79be6
mm: Use ptep/pmdp_set_numa() for updating _PAGE_NUMA bit
Yes. Having a testcase like this help
As small nit.
Please follow the existing subject format for all patches in here.
It will improve readability and also help understand these changes
better, later on.
mm/debug_vm_pgtable: <Specify changes to an individual test>
This makes sense. But could you please fold this code stanza with
the previous one in order to imply that 'ptep' did have some valid
entry before being cleared and checked against pte_none().
+ /*
+ * We should clear pte before we do set_pte_at
+ */
+ pte = ptep_get_and_clear(mm, vaddr, ptep);
pte = pte_mkyoung(pte);
set_pte_at(mm, vaddr, ptep, pte);
ptep_test_and_clear_young(vma, vaddr, ptep);
The comment above should also explain details that are mentioned
in the commit message i.e how platforms such as ppc64 expects a
clear pte entry for set_pte_at() to work.
I don't think it is specific to ppc64. There is nothing specific to
ppc64 architecture in there. It is an optimization used in kernel to
help architecture avoid TLB flush. I will update the comment as below
/* We should clear pte before we do set_pte_at so that set_pte_at don't
find a valid pte at ptep *?
is that good?
-aneesh
Problem is arch_ioremap_pmd_supported() symbol which should also be
explicitly included via <linux/io.h>, is not available without the
config CONFIG_HAVE_ARCH_HUGE_VMAP. ioremap_pmd_enabled() should have
been better here and has a fallback for !CONFIG_HAVE_ARCH_HUGE_VMAP.
But then the symbol is local to that file. Unless we would like to
make ioremap_pxx_enabled generally available, the remaining option
would be to wrap pxx_huge_tests() with CONFIG_HAVE_ARCH_HUGE_VMAP.
Similar changes should also be done for pud_huge_tests() as well.
Saved write support was added to track the write bit of a pte after marking the
pte protnone. This was done so that AUTONUMA can convert a write pte to protnone
and still track the old write bit. When converting it back we set the pte write
bit correctly thereby avoiding a write fault again. Hence enable the test only
when CONFIG_NUMA_BALANCING is enabled.
This has not been a problem on architectures that do not define pxx_savedwrite()
as they fallback on standard pxx_write() helpers. But ppc64 defines a fall back
for pte_savedwrite() when !CONFIG_NUMA_BALANCING, which always returns false.
Agreed, it makes sense to add CONFIG_NUMA_BALANCING check. But a simple runtime
check will do as the functions are always visible/defined.
if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
return;
Looks very similar to the previous patch, hence please fold it back.
Seems fair enough that pxx_set_at() expects given entry to be huge.
But would need to run them across enabled platforms to be sure.
set_pmd_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pmd_at() and hence expect it to be used to set locations
that are not a valid PTE.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
set_pud_at() should not be used to set a pte entry at locations that
already holds a valid pte entry. Architectures like ppc64 don't do TLB
invalidate in set_pud_at() and hence expect it to be used to set locations
that are not a valid PTE.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
Saved write support was added to track the write bit of a pte after marking the
pte protnone. This was done so that AUTONUMA can convert a write pte to protnone
and still track the old write bit. When converting it back we set the pte write
bit correctly thereby avoiding a write fault again.
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
As mentioned the test is broken. For hugetlb, the pgtable_t pages should
be allocated by huge_pte_alloc(). We need to hold huget_pte_lock()
before updating huge tlb pte. That takes hugepage size, which is mostly
derived out of vma. Hence vma need to be a hugetlb vma. Some of the
functions also depend on hstate. Also we should use set_huge_pte_at()
when setting up hugetlb pte entries. I was tempted to remove that test
completely marking it broken. But avoided that by marking it broken on
only PPC64.
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
As mentioned the test is broken. For hugetlb, the pgtable_t pages should be allocated by huge_pte_alloc(). We need to hold huget_pte_lock() before updating huge tlb pte. That takes hugepage size, which is mostly derived out of vma. Hence vma need to be a hugetlb vma. Some of the functions also depend on hstate. Also we should use set_huge_pte_at() when setting up hugetlb pte entries. I was tempted to remove that test completely marking it broken. But avoided that by marking it broken on only PPC64.
The test is not broken, hugetlb helpers on multiple platforms dont complain about
this at all. The tests here emulate 'enough' MM objects required for the helpers
on enabled platforms, to perform the primary task i.e page table transformation it
is expected to do. The test does not claim to emulate a perfect MM environment for
a given subsystem's (like HugeTLB) arch helpers. Now in this case, the MM objects
being emulated for the HugeTLB advanced tests does not seem to be sufficient for
ppc64 but it can be improved. But that does not mean it is broken in it's current
form for other platforms.
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
As mentioned the test is broken. For hugetlb, the pgtable_t pages should be allocated by huge_pte_alloc(). We need to hold huget_pte_lock() before updating huge tlb pte. That takes hugepage size, which is mostly derived out of vma. Hence vma need to be a hugetlb vma. Some of the functions also depend on hstate. Also we should use set_huge_pte_at() when setting up hugetlb pte entries. I was tempted to remove that test completely marking it broken. But avoided that by marking it broken on only PPC64.
The test is not broken, hugetlb helpers on multiple platforms dont complain about
this at all. The tests here emulate 'enough' MM objects required for the helpers
on enabled platforms, to perform the primary task i.e page table transformation it
is expected to do. The test does not claim to emulate a perfect MM environment for
a given subsystem's (like HugeTLB) arch helpers. Now in this case, the MM objects
being emulated for the HugeTLB advanced tests does not seem to be sufficient for
ppc64 but it can be improved. But that does not mean it is broken in it's current
form for other platforms.
There is nothing ppc64 specific here. It is just that we have
CONFIG_DEBUG_VM based checks for different possibly wrong usages of
these functions. This was done because we have different page sizes, two
different translations to support and we want to avoid any wrong usage.
IMHO expecting hugetlb page table helpers to work with a non hugetlb VMA
and without holding hugeTLB pte lock is a clear violation of hugetlb
interface.
-aneesh
@@ -1002,7 +1006,7 @@ static int __init debug_vm_pgtable(void)pgd_clear_tests(mm,pgdp);pte_advanced_tests(mm,vma,ptep,pte_aligned,vaddr,prot);-pmd_advanced_tests(mm,vma,pmdp,pmd_aligned,vaddr,prot);+pmd_advanced_tests(mm,vma,pmdp,pmd_aligned,vaddr,prot,saved_ptep);pud_advanced_tests(mm,vma,pudp,pud_aligned,vaddr,prot);hugetlb_advanced_tests(mm,vma,ptep,pte_aligned,vaddr,prot);
Makes sense, if it is required for THP to work correctly but needs to be tested
across enabled platforms. Why should not the same apply for pud_advanced_tests()
on platforms that supports PUD based THP.
@@ -564,7 +566,6 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, * This entry points to next level page table page. * Hence this must not qualify as pmd_bad(). */- pmd_clear(pmdp);
The tests do expect _PAGE_PTE bit set by different page table accessors.
This is not true for the kernel. Within the kernel, _PAGE_PTE bits are
usually set by set_pte_at(). To make the below tests work correctly add test
specific pfn_pte/pmd helpers that set _PAGE_PTE bit.
pte_t pte = pfn_pte(pfn, prot);
WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 65 +++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 24 deletions(-)
Again, no platform specific constructs please. This defeats the whole purpose of
this test. If __PAGE_PTE is required for the helpers, then pfn_pmd/pte() could
be modified to accommodate that. We dont see similar issues on other platforms,
hence could you please explain why ppc64 is different here.
The tests do expect _PAGE_PTE bit set by different page table accessors.
This is not true for the kernel. Within the kernel, _PAGE_PTE bits are
usually set by set_pte_at(). To make the below tests work correctly add test
specific pfn_pte/pmd helpers that set _PAGE_PTE bit.
pte_t pte = pfn_pte(pfn, prot);
WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 65 +++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 24 deletions(-)
Again, no platform specific constructs please. This defeats the whole purpose of
this test. If __PAGE_PTE is required for the helpers, then pfn_pmd/pte() could
be modified to accommodate that. We dont see similar issues on other platforms,
hence could you please explain why ppc64 is different here.
It is not platform specific. set_pte_at is the one that set the
_PAGE_PTE bit. We don't call that in the test. The test seems to make
the assumption that pfn_pte returns a proper pte which is not true.
-aneesh
@@ -1002,7 +1006,7 @@ static int __init debug_vm_pgtable(void)pgd_clear_tests(mm,pgdp);pte_advanced_tests(mm,vma,ptep,pte_aligned,vaddr,prot);-pmd_advanced_tests(mm,vma,pmdp,pmd_aligned,vaddr,prot);+pmd_advanced_tests(mm,vma,pmdp,pmd_aligned,vaddr,prot,saved_ptep);pud_advanced_tests(mm,vma,pudp,pud_aligned,vaddr,prot);hugetlb_advanced_tests(mm,vma,ptep,pte_aligned,vaddr,prot);
Makes sense, if it is required for THP to work correctly but needs to be tested
across enabled platforms. Why should not the same apply for pud_advanced_tests()
on platforms that supports PUD based THP.
pud doesn't have page table deposit/withdraw semantics. We use that to
support hugepage split. With pud mapping we don't split, we just drop
the hugepage and expect it to be faulted back in as regular page.
-aneesh
pmd_clear() should not be used to clear pmd level pte entries.
Could you please elaborate on this. The proposed change set does
not match the description here.
pmd_clear is implemented such that we don't use that to clear a huge pte
entry. We use pmdp_huge_get_and_clear() for that. Hence we have check in
pmd_clear which add a WARN if we find a _PAGE_PTE set on the entry.
In the test we follow a hugepmd usage with a pmd_clear. We should
instead at the end of the advanced pmd test use pmdp_huge_get_and_clear().
@@ -564,7 +566,6 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, * This entry points to next level page table page. * Hence this must not qualify as pmd_bad(). */- pmd_clear(pmdp);
The tests do expect _PAGE_PTE bit set by different page table accessors.
This is not true for the kernel. Within the kernel, _PAGE_PTE bits are
usually set by set_pte_at(). To make the below tests work correctly add test
specific pfn_pte/pmd helpers that set _PAGE_PTE bit.
pte_t pte = pfn_pte(pfn, prot);
WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
Signed-off-by: Aneesh Kumar K.V <redacted>
---
mm/debug_vm_pgtable.c | 65 +++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 24 deletions(-)
Again, no platform specific constructs please. This defeats the whole purpose of
this test. If __PAGE_PTE is required for the helpers, then pfn_pmd/pte() could
be modified to accommodate that. We dont see similar issues on other platforms,
hence could you please explain why ppc64 is different here.
It is not platform specific. set_pte_at is the one that set the _PAGE_PTE bit. We don't call that in the test. The test seems to make the assumption that pfn_pte returns a proper pte which is not true.
'#ifdef CONFIG_PPC_BOOK3S_64' definitely makes it platform specific. Here is how
set_pte_at() updates an entry on other platforms without changing the pte value.
_PAGE_PTE bit update during set_pte_at() on ppc64 seems to be a deviation.
1. set_pte_at() on arm64 directly update the entry after TLB, cache maintenance
2. set_pte_at() on s390 directly updates the entry for !CONFIG_PGSTE
3. set_pte_at() on arc directly updates the entry via set_pte()
4. set_pte_at() on x86 directly update the entry via native_set_pte()
set_pte_at() does take a pte created with pfn_pte().
As an example do_anonymous_page() does the same.
......
entry = mk_pte(page, vma->vm_page_prot); /* Call pfn_pte() */
entry = pte_sw_mkyoung(entry);
if (vma->vm_flags & VM_WRITE)
entry = pte_mkwrite(pte_mkdirty(entry));
.....
set_pte_at(vma->vm_mm, vmf->address, vmf->pte, entry);
.....
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
As mentioned the test is broken. For hugetlb, the pgtable_t pages should be allocated by huge_pte_alloc(). We need to hold huget_pte_lock() before updating huge tlb pte. That takes hugepage size, which is mostly derived out of vma. Hence vma need to be a hugetlb vma. Some of the functions also depend on hstate. Also we should use set_huge_pte_at() when setting up hugetlb pte entries. I was tempted to remove that test completely marking it broken. But avoided that by marking it broken on only PPC64.
The test is not broken, hugetlb helpers on multiple platforms dont complain about
this at all. The tests here emulate 'enough' MM objects required for the helpers
on enabled platforms, to perform the primary task i.e page table transformation it
is expected to do. The test does not claim to emulate a perfect MM environment for
a given subsystem's (like HugeTLB) arch helpers. Now in this case, the MM objects
being emulated for the HugeTLB advanced tests does not seem to be sufficient for
ppc64 but it can be improved. But that does not mean it is broken in it's current
form for other platforms.
There is nothing ppc64 specific here. It is just that we have CONFIG_DEBUG_VM based checks for different possibly wrong usages of these functions. This was done because we have different page sizes, two different translations to support and we want to avoid any wrong usage. IMHO expecting hugetlb page table helpers to work with a non hugetlb VMA and without holding hugeTLB pte lock is a clear violation of hugetlb interface.
Do you have a modified version of the test with HugeTLB marked VMA and with pte lock
held, which works on ppc664 ?
The seems to be missing quite a lot of details w.r.t allocating
the correct pgtable_t page (huge_pte_alloc()), holding the right
lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
Hence disable the test on ppc64.
This test is free from any platform specific #ifdefs which should
never be broken. If hugetlb_advanced_tests() does not work or is
not detailed enough for ppc64, then it would be great if you could
suggest some improvements so that it works for all enabled platforms.
As mentioned the test is broken. For hugetlb, the pgtable_t pages should be allocated by huge_pte_alloc(). We need to hold huget_pte_lock() before updating huge tlb pte. That takes hugepage size, which is mostly derived out of vma. Hence vma need to be a hugetlb vma. Some of the functions also depend on hstate. Also we should use set_huge_pte_at() when setting up hugetlb pte entries. I was tempted to remove that test completely marking it broken. But avoided that by marking it broken on only PPC64.
The test is not broken, hugetlb helpers on multiple platforms dont complain about
this at all. The tests here emulate 'enough' MM objects required for the helpers
on enabled platforms, to perform the primary task i.e page table transformation it
is expected to do. The test does not claim to emulate a perfect MM environment for
a given subsystem's (like HugeTLB) arch helpers. Now in this case, the MM objects
being emulated for the HugeTLB advanced tests does not seem to be sufficient for
ppc64 but it can be improved. But that does not mean it is broken in it's current
form for other platforms.
There is nothing ppc64 specific here. It is just that we have CONFIG_DEBUG_VM based checks for different possibly wrong usages of these functions. This was done because we have different page sizes, two different translations to support and we want to avoid any wrong usage. IMHO expecting hugetlb page table helpers to work with a non hugetlb VMA and without holding hugeTLB pte lock is a clear violation of hugetlb interface.
Do you have a modified version of the test with HugeTLB marked VMA and with pte lock
held, which works on ppc664 ?
Nope. That is one of the reason I commented that out. We can sort that
out slowly.
-aneesh