* Re: [PATCH 4/4] powerpc/64s/radix: Fix mm_cpumask trimming race vs kthread_use_mm
From: Nicholas Piggin @ 2020-09-02 9:48 UTC (permalink / raw)
To: linux-mm, Michael Ellerman
Cc: Jens Axboe, linux-arch, Peter Zijlstra, Aneesh Kumar K.V,
linux-kernel, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <87pn751zcb.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of September 1, 2020 10:00 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Commit 0cef77c7798a7 ("powerpc/64s/radix: flush remote CPUs out of
>> single-threaded mm_cpumask") added a mechanism to trim the mm_cpumask of
>> a process under certain conditions. One of the assumptions is that
>> mm_users would not be incremented via a reference outside the process
>> context with mmget_not_zero() then go on to kthread_use_mm() via that
>> reference.
>>
>> That invariant was broken by io_uring code (see previous sparc64 fix),
>> but I'll point Fixes: to the original powerpc commit because we are
>> changing that assumption going forward, so this will make backports
>> match up.
>>
>> Fix this by no longer relying on that assumption, but by having each CPU
>> check the mm is not being used, and clearing their own bit from the mask
>> if it's okay. This fix relies on commit 38cf307c1f20 ("mm: fix
>> kthread_use_mm() vs TLB invalidate") to disable irqs over the mm switch,
>> and ARCH_WANT_IRQS_OFF_ACTIVATE_MM to be enabled.
>
> You could use:
>
> Depends-on: 38cf307c1f20 ("mm: fix kthread_use_mm() vs TLB invalidate")
Good idea I wil.
>> Fixes: 0cef77c7798a7 ("powerpc/64s/radix: flush remote CPUs out of single-threaded mm_cpumask")
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/tlb.h | 13 -------------
>> arch/powerpc/mm/book3s64/radix_tlb.c | 23 ++++++++++++++++-------
>> 2 files changed, 16 insertions(+), 20 deletions(-)
>
> One minor nit below if you're respinning anyway.
>
> You know this stuff better than me, but I still reviewed it and it seems
> good to me.
>
> Reviewed-by: Michael Ellerman <mpe@ellerman.id.au>
Thanks.
>
>> diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
>> index fbc6f3002f23..d97f061fecac 100644
>> --- a/arch/powerpc/include/asm/tlb.h
>> +++ b/arch/powerpc/include/asm/tlb.h
>> @@ -66,19 +66,6 @@ static inline int mm_is_thread_local(struct mm_struct *mm)
>> return false;
>> return cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm));
>> }
>> -static inline void mm_reset_thread_local(struct mm_struct *mm)
>> -{
>> - WARN_ON(atomic_read(&mm->context.copros) > 0);
>> - /*
>> - * It's possible for mm_access to take a reference on mm_users to
>> - * access the remote mm from another thread, but it's not allowed
>> - * to set mm_cpumask, so mm_users may be > 1 here.
>> - */
>> - WARN_ON(current->mm != mm);
>> - atomic_set(&mm->context.active_cpus, 1);
>> - cpumask_clear(mm_cpumask(mm));
>> - cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm));
>> -}
>> #else /* CONFIG_PPC_BOOK3S_64 */
>> static inline int mm_is_thread_local(struct mm_struct *mm)
>> {
>> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
>> index 0d233763441f..a421a0e3f930 100644
>> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
>> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
>> @@ -645,19 +645,29 @@ static void do_exit_flush_lazy_tlb(void *arg)
>> struct mm_struct *mm = arg;
>> unsigned long pid = mm->context.id;
>>
>> + /*
>> + * A kthread could have done a mmget_not_zero() after the flushing CPU
>> + * checked mm_users == 1, and be in the process of kthread_use_mm when
> ^
> in mm_is_singlethreaded()
>
> Adding that reference would help join the dots for a new reader I think.
Yes you're right I can change that.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/vdso32: link vdso64 with linker
From: Christophe Leroy @ 2020-09-02 10:16 UTC (permalink / raw)
To: Nick Desaulniers, Michael Ellerman, Nicholas Piggin
Cc: Joe Lawrence, Kees Cook, Fangrui Song, linux-kernel,
clang-built-linux, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200901222523.1941988-3-ndesaulniers@google.com>
On 9/1/20 10:25 PM, Nick Desaulniers wrote:
> Rather than invoke the compiler as the driver, use the linker. That way
> we can check --orphan-handling=warn support correctly, as cc-ldoption
> was removed in
> commit 055efab3120b ("kbuild: drop support for cc-ldoption").
>
> Requires dropping the .got section. I couldn't find how it was used in
> the vdso32.
>
> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
> Link: https://lore.kernel.org/lkml/CAKwvOdnn3wxYdJomvnveyD_njwRku3fABWT_bS92duihhywLJQ@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Not sure removing .got is a good idea or not. Otherwise I observe the
> following link error:
> powerpc-linux-gnu-ld: warning: orphan section `.got' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.got'
> powerpc-linux-gnu-ld: _GLOBAL_OFFSET_TABLE_ not defined in linker created .got
> powerpc-linux-gnu-ld: final link failed: bad value
Finally I spotted it I think:
make arch/powerpc/kernel/vdso32/ V=1
powerpc64-linux-ld -EB -m elf64ppc -shared -soname linux-vdso32.so.1
--eh-frame-hdr --orphan-handling=warn -T
arch/powerpc/kernel/vdso32/vdso32.lds
arch/powerpc/kernel/vdso32/sigtramp.o
arch/powerpc/kernel/vdso32/gettimeofday.o
arch/powerpc/kernel/vdso32/datapage.o
arch/powerpc/kernel/vdso32/cacheflush.o
arch/powerpc/kernel/vdso32/note.o arch/powerpc/kernel/vdso32/getcpu.o -o
arch/powerpc/kernel/vdso32/vdso32.so.dbg
If I do the same manually but with -m elf32ppc instead of -m elf64ppc,
there is no failure.
Adding -m elf32ppc to ldflags-y also works, allthough I don't like too
much having "-m elf64ppc -m elf32ppc" on the line.
Christophe
^ permalink raw reply
* [Bug 209029] kernel 5.9-rc2 fails to boot on a PowerMac G5 11,2 - BUG: Kernel NULL pointer dereference on read at 0x00000020
From: bugzilla-daemon @ 2020-09-02 10:48 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209029-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209029
--- Comment #4 from Erhard F. (erhard_f@mailbox.org) ---
(In reply to Christophe Leroy from comment #3)
> Did you try without CONFIG_DEBUG_VM_PGTABLE ?
Without CONFIG_DEBUG_VM_PGTABLE the G5 boots fine. Thanks!
> If you want CONFIG_DEBUG_VM_PGTABLE, the following series aims at fixing it
> for PPC64:
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=197961
Did not check the series as current ozlabs patches indicate that the
CONFIG_DEBUG_VM_PGTABLE option is removed for the time being.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH v4 00/13] mm/debug_vm_pgtable fixes
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
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.
These tests are broken w.r.t page table update rules and results in kernel
crash as below.
[ 21.083519] kernel BUG at arch/powerpc/mm/pgtable.c:304!
cpu 0x0: Vector: 700 (Program Check) at [c000000c6d1e76c0]
pc: c00000000009a5ec: assert_pte_locked+0x14c/0x380
lr: c0000000005eeeec: pte_update+0x11c/0x190
sp: c000000c6d1e7950
msr: 8000000002029033
current = 0xc000000c6d172c80
paca = 0xc000000003ba0000 irqmask: 0x03 irq_happened: 0x01
pid = 1, comm = swapper/0
kernel BUG at arch/powerpc/mm/pgtable.c:304!
[link register ] c0000000005eeeec pte_update+0x11c/0x190
[c000000c6d1e7950] 0000000000000001 (unreliable)
[c000000c6d1e79b0] c0000000005eee14 pte_update+0x44/0x190
[c000000c6d1e7a10] c000000001a2ca9c pte_advanced_tests+0x160/0x3d8
[c000000c6d1e7ab0] c000000001a2d4fc debug_vm_pgtable+0x7e8/0x1338
[c000000c6d1e7ba0] c0000000000116ec do_one_initcall+0xac/0x5f0
[c000000c6d1e7c80] c0000000019e4fac kernel_init_freeable+0x4dc/0x5a4
[c000000c6d1e7db0] c000000000012474 kernel_init+0x24/0x160
[c000000c6d1e7e20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
With DEBUG_VM disabled
[ 20.530152] BUG: Kernel NULL pointer dereference on read at 0x00000000
[ 20.530183] Faulting instruction address: 0xc0000000000df330
cpu 0x33: Vector: 380 (Data SLB Access) at [c000000c6d19f700]
pc: c0000000000df330: memset+0x68/0x104
lr: c00000000009f6d8: hash__pmdp_huge_get_and_clear+0xe8/0x1b0
sp: c000000c6d19f990
msr: 8000000002009033
dar: 0
current = 0xc000000c6d177480
paca = 0xc00000001ec4f400 irqmask: 0x03 irq_happened: 0x01
pid = 1, comm = swapper/0
[link register ] c00000000009f6d8 hash__pmdp_huge_get_and_clear+0xe8/0x1b0
[c000000c6d19f990] c00000000009f748 hash__pmdp_huge_get_and_clear+0x158/0x1b0 (unreliable)
[c000000c6d19fa10] c0000000019ebf30 pmd_advanced_tests+0x1f0/0x378
[c000000c6d19fab0] c0000000019ed088 debug_vm_pgtable+0x79c/0x1244
[c000000c6d19fba0] c0000000000116ec do_one_initcall+0xac/0x5f0
[c000000c6d19fc80] c0000000019a4fac kernel_init_freeable+0x4dc/0x5a4
[c000000c6d19fdb0] c000000000012474 kernel_init+0x24/0x160
[c000000c6d19fe20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
Changes from v3:
* Address review feedback
* Move page table depost and withdraw patch after adding pmdlock to avoid bisect failure.
Changes from v2:
* Fix build failure with different configs and architecture.
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/locks: Move non page table modifying test together
mm/debug_vm_pgtable/locks: Take correct page table lock
mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
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: Avoid none pte in pte_clear_test
arch/powerpc/include/asm/book3s/64/pgtable.h | 29 +++-
arch/powerpc/include/asm/nohash/pgtable.h | 5 -
arch/powerpc/mm/pgtable.c | 5 -
mm/debug_vm_pgtable.c | 171 ++++++++++++-------
4 files changed, 131 insertions(+), 79 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH v4 02/13] powerpc/mm: Move setting pte specific flags to pfn_pte
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
powerpc used to set the pte specific flags in set_pte_at(). This is
different from other architectures. To be consistent with other
architecture update pfn_pte to set _PAGE_PTE on ppc64. Also, drop now
unused pte_mkpte.
We add a VM_WARN_ON() to catch the usage of calling set_pte_at()
without setting _PAGE_PTE bit. We will remove that after a few releases.
With respect to huge pmd entries, pmd_mkhuge() takes care of adding the
_PAGE_PTE bit.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 15 +++++++++------
arch/powerpc/include/asm/nohash/pgtable.h | 5 -----
arch/powerpc/mm/pgtable.c | 5 -----
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 079211968987..2382fd516f6b 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -619,7 +619,7 @@ static inline pte_t pfn_pte(unsigned long pfn, pgprot_t pgprot)
VM_BUG_ON(pfn >> (64 - PAGE_SHIFT));
VM_BUG_ON((pfn << PAGE_SHIFT) & ~PTE_RPN_MASK);
- return __pte(((pte_basic_t)pfn << PAGE_SHIFT) | pgprot_val(pgprot));
+ return __pte(((pte_basic_t)pfn << PAGE_SHIFT) | pgprot_val(pgprot) | _PAGE_PTE);
}
static inline unsigned long pte_pfn(pte_t pte)
@@ -655,11 +655,6 @@ static inline pte_t pte_mkexec(pte_t pte)
return __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_EXEC));
}
-static inline pte_t pte_mkpte(pte_t pte)
-{
- return __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_PTE));
-}
-
static inline pte_t pte_mkwrite(pte_t pte)
{
/*
@@ -823,6 +818,14 @@ static inline int pte_none(pte_t pte)
static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, int percpu)
{
+
+ VM_WARN_ON(!(pte_raw(pte) & cpu_to_be64(_PAGE_PTE)));
+ /*
+ * Keep the _PAGE_PTE added till we are sure we handle _PAGE_PTE
+ * in all the callers.
+ */
+ pte = __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_PTE));
+
if (radix_enabled())
return radix__set_pte_at(mm, addr, ptep, pte, percpu);
return hash__set_pte_at(mm, addr, ptep, pte, percpu);
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h
index 4b7c3472eab1..6277e7596ae5 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -140,11 +140,6 @@ static inline pte_t pte_mkold(pte_t pte)
return __pte(pte_val(pte) & ~_PAGE_ACCESSED);
}
-static inline pte_t pte_mkpte(pte_t pte)
-{
- return pte;
-}
-
static inline pte_t pte_mkspecial(pte_t pte)
{
return __pte(pte_val(pte) | _PAGE_SPECIAL);
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index 9c0547d77af3..ab57b07ef39a 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -184,9 +184,6 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
*/
VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
- /* Add the pte bit when trying to set a pte */
- pte = pte_mkpte(pte);
-
/* Note: mm->context.id might not yet have been assigned as
* this context might not have been activated yet when this
* is called.
@@ -275,8 +272,6 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_
*/
VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
- pte = pte_mkpte(pte);
-
pte = set_pte_filter(pte);
val = pte_val(pte);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 01/13] powerpc/mm: Add DEBUG_VM WARN for pmd_clear
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
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 <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 6de56c3b33c4..079211968987 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -868,6 +868,13 @@ static inline bool pte_ci(pte_t pte)
static inline void pmd_clear(pmd_t *pmdp)
{
+ if (IS_ENABLED(CONFIG_DEBUG_VM) && !radix_enabled()) {
+ /*
+ * Don't use this if we can possibly have a hash page table
+ * entry mapping this.
+ */
+ WARN_ON((pmd_val(*pmdp) & (H_PAGE_HASHPTE | _PAGE_PTE)) == (H_PAGE_HASHPTE | _PAGE_PTE));
+ }
*pmdp = __pmd(0);
}
@@ -916,6 +923,13 @@ static inline int pmd_bad(pmd_t pmd)
static inline void pud_clear(pud_t *pudp)
{
+ if (IS_ENABLED(CONFIG_DEBUG_VM) && !radix_enabled()) {
+ /*
+ * Don't use this if we can possibly have a hash page table
+ * entry mapping this.
+ */
+ WARN_ON((pud_val(*pudp) & (H_PAGE_HASHPTE | _PAGE_PTE)) == (H_PAGE_HASHPTE | _PAGE_PTE));
+ }
*pudp = __pud(0);
}
--
2.26.2
^ permalink raw reply related
* [PATCH v4 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
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 <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 086309fb9b6f..00649b47f6e0 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -44,10 +44,17 @@
* entry type. But these bits might affect the ability to clear entries with
* 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.
+ * 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 S390_SKIP_MASK GENMASK(3, 0)
+#if __BITS_PER_LONG == 64
+#define PPC64_SKIP_MASK GENMASK(62, 62)
+#else
+#define PPC64_SKIP_MASK 0x0
+#endif
+#define ARCH_SKIP_MASK (S390_SKIP_MASK | PPC64_SKIP_MASK)
+#define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
#define RANDOM_NZVALUE GENMASK(7, 0)
static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
--
2.26.2
^ permalink raw reply related
* [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
ppc64 supports huge vmap only with radix translation. Hence use arch helper
to determine the huge vmap support.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 00649b47f6e0..4c73e63b4ceb 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -28,6 +28,7 @@
#include <linux/swapops.h>
#include <linux/start_kernel.h>
#include <linux/sched/mm.h>
+#include <linux/io.h>
#include <asm/pgalloc.h>
#include <asm/tlbflush.h>
@@ -206,11 +207,12 @@ 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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
+ if (!arch_ioremap_pmd_supported())
return;
pr_debug("Validating PMD huge\n");
@@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
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)
{
@@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
+ if (!arch_ioremap_pud_supported())
return;
pr_debug("Validating PUD huge\n");
@@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
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(unsigned long pfn, pgprot_t prot) { }
static void __init pud_advanced_tests(struct mm_struct *mm,
--
2.26.2
^ permalink raw reply related
* [PATCH v4 05/13] mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with CONFIG_NUMA_BALANCING
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
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 and
use protnone protflags.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 4c73e63b4ceb..8704901f6bd8 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -119,10 +119,14 @@ static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot)
{
pte_t pte = pfn_pte(pfn, prot);
+ if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
+ return;
+
pr_debug("Validating PTE saved write\n");
WARN_ON(!pte_savedwrite(pte_mk_savedwrite(pte_clear_savedwrite(pte))));
WARN_ON(pte_savedwrite(pte_clear_savedwrite(pte_mk_savedwrite(pte))));
}
+
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
{
@@ -234,6 +238,9 @@ static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
{
pmd_t pmd = pfn_pmd(pfn, prot);
+ if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
+ return;
+
pr_debug("Validating PMD saved write\n");
WARN_ON(!pmd_savedwrite(pmd_mk_savedwrite(pmd_clear_savedwrite(pmd))));
WARN_ON(pmd_savedwrite(pmd_clear_savedwrite(pmd_mk_savedwrite(pmd))));
@@ -1019,8 +1026,8 @@ static int __init debug_vm_pgtable(void)
pmd_huge_tests(pmdp, pmd_aligned, prot);
pud_huge_tests(pudp, pud_aligned, prot);
- pte_savedwrite_tests(pte_aligned, prot);
- pmd_savedwrite_tests(pmd_aligned, prot);
+ pte_savedwrite_tests(pte_aligned, protnone);
+ pmd_savedwrite_tests(pmd_aligned, protnone);
pte_unmap_unlock(ptep, ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 06/13] mm/debug_vm_pgtable/THP: Mark the pte entry huge before using set_pmd/pud_at
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
kernel expects entries to be marked huge before we use
set_pmd_at()/set_pud_at().
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 8704901f6bd8..9cafed39c236 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -155,7 +155,7 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
unsigned long pfn, unsigned long vaddr,
pgprot_t prot)
{
- pmd_t pmd = pfn_pmd(pfn, prot);
+ pmd_t pmd;
if (!has_transparent_hugepage())
return;
@@ -164,19 +164,19 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
/* Align the address wrt HPAGE_PMD_SIZE */
vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
- pmd = pfn_pmd(pfn, prot);
+ pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_set_wrprotect(mm, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_write(pmd));
- pmd = pfn_pmd(pfn, prot);
+ pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_huge_get_and_clear(mm, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(!pmd_none(pmd));
- pmd = pfn_pmd(pfn, prot);
+ pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
pmd = pmd_wrprotect(pmd);
pmd = pmd_mkclean(pmd);
set_pmd_at(mm, vaddr, pmdp, pmd);
@@ -236,7 +236,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;
@@ -276,7 +276,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;
@@ -285,25 +285,27 @@ 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 = pud_mkhuge(pfn_pud(pfn, prot));
set_pud_at(mm, vaddr, pudp, pud);
pudp_set_wrprotect(mm, vaddr, pudp);
pud = READ_ONCE(*pudp);
WARN_ON(pud_write(pud));
#ifndef __PAGETABLE_PMD_FOLDED
- pud = pfn_pud(pfn, prot);
+ pud = pud_mkhuge(pfn_pud(pfn, prot));
set_pud_at(mm, vaddr, pudp, pud);
pudp_huge_get_and_clear(mm, vaddr, pudp);
pud = READ_ONCE(*pudp);
WARN_ON(!pud_none(pud));
- pud = pfn_pud(pfn, prot);
+ pud = pud_mkhuge(pfn_pud(pfn, prot));
set_pud_at(mm, vaddr, pudp, pud);
pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
pud = READ_ONCE(*pudp);
WARN_ON(!pud_none(pud));
#endif /* __PAGETABLE_PMD_FOLDED */
- pud = pfn_pud(pfn, prot);
+
+ pud = pud_mkhuge(pfn_pud(pfn, prot));
pud = pud_wrprotect(pud);
pud = pud_mkclean(pud);
set_pud_at(mm, vaddr, pudp, pud);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 07/13] mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an existing pte entry
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
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 <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 9cafed39c236..de333871f407 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -79,15 +79,18 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
{
pte_t pte = pfn_pte(pfn, prot);
+ /*
+ * Architectures optimize set_pte_at by avoiding TLB flush.
+ * This requires set_pte_at to be not used to update an
+ * existing pte entry. Clear pte before we do set_pte_at
+ */
+
pr_debug("Validating PTE advanced\n");
pte = pfn_pte(pfn, prot);
set_pte_at(mm, vaddr, ptep, pte);
ptep_set_wrprotect(mm, vaddr, ptep);
pte = ptep_get(ptep);
WARN_ON(pte_write(pte));
-
- pte = pfn_pte(pfn, prot);
- set_pte_at(mm, vaddr, ptep, pte);
ptep_get_and_clear(mm, vaddr, ptep);
pte = ptep_get(ptep);
WARN_ON(!pte_none(pte));
@@ -101,13 +104,11 @@ static void __init pte_advanced_tests(struct mm_struct *mm,
ptep_set_access_flags(vma, vaddr, ptep, pte, 1);
pte = ptep_get(ptep);
WARN_ON(!(pte_write(pte) && pte_dirty(pte)));
-
- pte = pfn_pte(pfn, prot);
- set_pte_at(mm, vaddr, ptep, pte);
ptep_get_and_clear_full(mm, vaddr, ptep, 1);
pte = ptep_get(ptep);
WARN_ON(!pte_none(pte));
+ pte = pfn_pte(pfn, prot);
pte = pte_mkyoung(pte);
set_pte_at(mm, vaddr, ptep, pte);
ptep_test_and_clear_young(vma, vaddr, ptep);
@@ -169,9 +170,6 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmdp_set_wrprotect(mm, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_write(pmd));
-
- pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
- set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_huge_get_and_clear(mm, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(!pmd_none(pmd));
@@ -185,13 +183,11 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1);
pmd = READ_ONCE(*pmdp);
WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd)));
-
- pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
- set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1);
pmd = READ_ONCE(*pmdp);
WARN_ON(!pmd_none(pmd));
+ pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
pmd = pmd_mkyoung(pmd);
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_test_and_clear_young(vma, vaddr, pmdp);
@@ -292,17 +288,9 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
WARN_ON(pud_write(pud));
#ifndef __PAGETABLE_PMD_FOLDED
- pud = pud_mkhuge(pfn_pud(pfn, prot));
- set_pud_at(mm, vaddr, pudp, pud);
pudp_huge_get_and_clear(mm, vaddr, pudp);
pud = READ_ONCE(*pudp);
WARN_ON(!pud_none(pud));
-
- pud = pud_mkhuge(pfn_pud(pfn, prot));
- set_pud_at(mm, vaddr, pudp, pud);
- pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
- pud = READ_ONCE(*pudp);
- WARN_ON(!pud_none(pud));
#endif /* __PAGETABLE_PMD_FOLDED */
pud = pud_mkhuge(pfn_pud(pfn, prot));
@@ -315,6 +303,13 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
pud = READ_ONCE(*pudp);
WARN_ON(!(pud_write(pud) && pud_dirty(pud)));
+#ifndef __PAGETABLE_PMD_FOLDED
+ pudp_huge_get_and_clear_full(mm, vaddr, pudp, 1);
+ pud = READ_ONCE(*pudp);
+ WARN_ON(!pud_none(pud));
+#endif /* __PAGETABLE_PMD_FOLDED */
+
+ pud = pud_mkhuge(pfn_pud(pfn, prot));
pud = pud_mkyoung(pud);
set_pud_at(mm, vaddr, pudp, pud);
pudp_test_and_clear_young(vma, vaddr, pudp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 08/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
This will help in adding proper locks in a later patch
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 51 ++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index de333871f407..f59cf6a9b05e 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -986,7 +986,7 @@ static int __init debug_vm_pgtable(void)
p4dp = p4d_alloc(mm, pgdp, vaddr);
pudp = pud_alloc(mm, p4dp, vaddr);
pmdp = pmd_alloc(mm, pudp, vaddr);
- ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
+ ptep = pte_alloc_map(mm, pmdp, vaddr);
/*
* Save all the page table page addresses as the page table
@@ -1006,33 +1006,12 @@ static int __init debug_vm_pgtable(void)
p4d_basic_tests(p4d_aligned, prot);
pgd_basic_tests(pgd_aligned, prot);
- pte_clear_tests(mm, ptep, vaddr);
- pmd_clear_tests(mm, pmdp);
- pud_clear_tests(mm, pudp);
- p4d_clear_tests(mm, p4dp);
- 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);
- pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
- hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
-
pmd_leaf_tests(pmd_aligned, prot);
pud_leaf_tests(pud_aligned, prot);
- pmd_huge_tests(pmdp, pmd_aligned, prot);
- pud_huge_tests(pudp, pud_aligned, prot);
-
pte_savedwrite_tests(pte_aligned, protnone);
pmd_savedwrite_tests(pmd_aligned, protnone);
- pte_unmap_unlock(ptep, ptl);
-
- pmd_populate_tests(mm, pmdp, saved_ptep);
- pud_populate_tests(mm, pudp, saved_pmdp);
- p4d_populate_tests(mm, p4dp, saved_pudp);
- pgd_populate_tests(mm, pgdp, saved_p4dp);
-
pte_special_tests(pte_aligned, prot);
pte_protnone_tests(pte_aligned, protnone);
pmd_protnone_tests(pmd_aligned, protnone);
@@ -1050,11 +1029,37 @@ static int __init debug_vm_pgtable(void)
pmd_swap_tests(pmd_aligned, prot);
swap_migration_tests();
- hugetlb_basic_tests(pte_aligned, prot);
pmd_thp_tests(pmd_aligned, prot);
pud_thp_tests(pud_aligned, prot);
+ hugetlb_basic_tests(pte_aligned, prot);
+
+ pte_clear_tests(mm, ptep, vaddr);
+ pmd_clear_tests(mm, pmdp);
+ pud_clear_tests(mm, pudp);
+ p4d_clear_tests(mm, p4dp);
+ pgd_clear_tests(mm, pgdp);
+
+ ptl = pte_lockptr(mm, pmdp);
+ spin_lock(ptl);
+
+ pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
+ pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
+ hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+
+
+ pmd_huge_tests(pmdp, pmd_aligned, prot);
+ pud_huge_tests(pudp, pud_aligned, prot);
+
+ pte_unmap_unlock(ptep, ptl);
+
+ pmd_populate_tests(mm, pmdp, saved_ptep);
+ pud_populate_tests(mm, pudp, saved_pmdp);
+ p4d_populate_tests(mm, p4dp, saved_pudp);
+ pgd_populate_tests(mm, pgdp, saved_p4dp);
+
p4d_free(mm, saved_p4dp);
pud_free(mm, saved_pudp);
pmd_free(mm, saved_pmdp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 09/13] mm/debug_vm_pgtable/locks: Take correct page table lock
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
Make sure we call pte accessors with correct lock held.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index f59cf6a9b05e..2bc1952e5f83 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -1035,30 +1035,39 @@ static int __init debug_vm_pgtable(void)
hugetlb_basic_tests(pte_aligned, prot);
- pte_clear_tests(mm, ptep, vaddr);
- pmd_clear_tests(mm, pmdp);
- pud_clear_tests(mm, pudp);
- p4d_clear_tests(mm, p4dp);
- pgd_clear_tests(mm, pgdp);
+ /*
+ * Page table modifying tests. They need to hold
+ * proper page table lock.
+ */
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
-
+ pte_clear_tests(mm, ptep, vaddr);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
- pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
- pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
- hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
-
+ pte_unmap_unlock(ptep, ptl);
+ ptl = pmd_lock(mm, pmdp);
+ pmd_clear_tests(mm, pmdp);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
pmd_huge_tests(pmdp, pmd_aligned, prot);
+ pmd_populate_tests(mm, pmdp, saved_ptep);
+ spin_unlock(ptl);
+
+ ptl = pud_lock(mm, pudp);
+ pud_clear_tests(mm, pudp);
+ pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot);
pud_huge_tests(pudp, pud_aligned, prot);
+ pud_populate_tests(mm, pudp, saved_pmdp);
+ spin_unlock(ptl);
- pte_unmap_unlock(ptep, ptl);
+ hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
- pmd_populate_tests(mm, pmdp, saved_ptep);
- pud_populate_tests(mm, pudp, saved_pmdp);
+ spin_lock(&mm->page_table_lock);
+ p4d_clear_tests(mm, p4dp);
+ pgd_clear_tests(mm, pgdp);
p4d_populate_tests(mm, p4dp, saved_pudp);
pgd_populate_tests(mm, pgdp, saved_p4dp);
+ spin_unlock(&mm->page_table_lock);
p4d_free(mm, saved_p4dp);
pud_free(mm, saved_pudp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 10/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
Architectures like ppc64 use deposited page table while updating the
huge pte entries.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 2bc1952e5f83..26023d990bd0 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -154,7 +154,7 @@ static void __init pmd_basic_tests(unsigned long pfn, pgprot_t prot)
static void __init pmd_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma, pmd_t *pmdp,
unsigned long pfn, unsigned long vaddr,
- pgprot_t prot)
+ pgprot_t prot, pgtable_t pgtable)
{
pmd_t pmd;
@@ -165,6 +165,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
/* Align the address wrt HPAGE_PMD_SIZE */
vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
+ pgtable_trans_huge_deposit(mm, pmdp, pgtable);
+
pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
set_pmd_at(mm, vaddr, pmdp, pmd);
pmdp_set_wrprotect(mm, vaddr, pmdp);
@@ -193,6 +195,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmdp_test_and_clear_young(vma, vaddr, pmdp);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_young(pmd));
+
+ pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
}
static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
@@ -371,7 +375,7 @@ static void __init pud_basic_tests(unsigned long pfn, pgprot_t prot) { }
static void __init pmd_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma, pmd_t *pmdp,
unsigned long pfn, unsigned long vaddr,
- pgprot_t prot)
+ pgprot_t prot, pgtable_t pgtable)
{
}
static void __init pud_advanced_tests(struct mm_struct *mm,
@@ -1048,7 +1052,7 @@ static int __init debug_vm_pgtable(void)
ptl = pmd_lock(mm, pmdp);
pmd_clear_tests(mm, pmdp);
- pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot);
+ pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep);
pmd_huge_tests(pmdp, pmd_aligned, prot);
pmd_populate_tests(mm, pmdp, saved_ptep);
spin_unlock(ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 11/13] mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pmd_clear() should not be used to clear pmd level pte entries.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 26023d990bd0..b53903fdee85 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -196,6 +196,8 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_young(pmd));
+ /* Clear the pte entries */
+ pmdp_huge_get_and_clear(mm, vaddr, pmdp);
pgtable = pgtable_trans_huge_withdraw(mm, pmdp);
}
@@ -319,6 +321,8 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
pudp_test_and_clear_young(vma, vaddr, pudp);
pud = READ_ONCE(*pudp);
WARN_ON(pud_young(pud));
+
+ pudp_huge_get_and_clear(mm, vaddr, pudp);
}
static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
@@ -442,8 +446,6 @@ static void __init pud_populate_tests(struct mm_struct *mm, pud_t *pudp,
* This entry points to next level page table page.
* Hence this must not qualify as pud_bad().
*/
- pmd_clear(pmdp);
- pud_clear(pudp);
pud_populate(mm, pudp, pmdp);
pud = READ_ONCE(*pudp);
WARN_ON(pud_bad(pud));
@@ -575,7 +577,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);
pmd_populate(mm, pmdp, pgtable);
pmd = READ_ONCE(*pmdp);
WARN_ON(pmd_bad(pmd));
--
2.26.2
^ permalink raw reply related
* [PATCH v4 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
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 <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index b53903fdee85..9afa1354326b 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -811,6 +811,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
#endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
}
+#ifndef CONFIG_PPC_BOOK3S_64
static void __init hugetlb_advanced_tests(struct mm_struct *mm,
struct vm_area_struct *vma,
pte_t *ptep, unsigned long pfn,
@@ -853,6 +854,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
pte = huge_ptep_get(ptep);
WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
}
+#endif
#else /* !CONFIG_HUGETLB_PAGE */
static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
static void __init hugetlb_advanced_tests(struct mm_struct *mm,
@@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
pud_populate_tests(mm, pudp, saved_pmdp);
spin_unlock(ptl);
+#ifndef CONFIG_PPC_BOOK3S_64
hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
+#endif
spin_lock(&mm->page_table_lock);
p4d_clear_tests(mm, p4dp);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test
From: Aneesh Kumar K.V @ 2020-09-02 11:42 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pte_clear_tests operate on an existing pte entry. Make sure that
is not a none pte entry.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 9afa1354326b..c36530c69e33 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -542,9 +542,10 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
#endif /* PAGETABLE_P4D_FOLDED */
static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
- unsigned long vaddr)
+ unsigned long pfn, unsigned long vaddr,
+ pgprot_t prot)
{
- pte_t pte = ptep_get(ptep);
+ pte_t pte = pfn_pte(pfn, prot);
pr_debug("Validating PTE clear\n");
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
@@ -1049,7 +1050,7 @@ static int __init debug_vm_pgtable(void)
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
- pte_clear_tests(mm, ptep, vaddr);
+ pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
pte_unmap_unlock(ptep, ptl);
--
2.26.2
^ permalink raw reply related
* [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test
From: Aneesh Kumar K.V @ 2020-09-02 11:46 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: linuxppc-dev, Aneesh Kumar K.V, Anshuman Khandual
In-Reply-To: <20200902114222.181353-1-aneesh.kumar@linux.ibm.com>
pte_clear_tests operate on an existing pte entry. Make sure that
is not a none pte entry.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
mm/debug_vm_pgtable.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 9afa1354326b..c36530c69e33 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -542,9 +542,10 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
#endif /* PAGETABLE_P4D_FOLDED */
static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
- unsigned long vaddr)
+ unsigned long pfn, unsigned long vaddr,
+ pgprot_t prot)
{
- pte_t pte = ptep_get(ptep);
+ pte_t pte = pfn_pte(pfn, prot);
pr_debug("Validating PTE clear\n");
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
@@ -1049,7 +1050,7 @@ static int __init debug_vm_pgtable(void)
ptl = pte_lockptr(mm, pmdp);
spin_lock(ptl);
- pte_clear_tests(mm, ptep, vaddr);
+ pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot);
pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
pte_unmap_unlock(ptep, ptl);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
From: Michael Ellerman @ 2020-09-02 12:14 UTC (permalink / raw)
To: Nick Desaulniers, Nicholas Piggin
Cc: Christophe Leroy, Joe Lawrence, Kees Cook, Fangrui Song,
Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20200901222523.1941988-2-ndesaulniers@google.com>
Nick Desaulniers <ndesaulniers@google.com> writes:
> Rather than invoke the compiler as the driver, use the linker. That way
> we can check --orphan-handling=warn support correctly, as cc-ldoption
> was removed in
> commit 055efab3120b ("kbuild: drop support for cc-ldoption").
Ouch.
Seems make is quite happy to $(call deadbeef, ...) and not print a
warning, which I guess is probably a feature.
> Painstakingly compared the output between `objdump -a` before and after
> this change. Now function symbols have the correct type of FUNC rather
> than NONE, and the entry is slightly different (which doesn't matter for
> the vdso). Binary size is the same.
>
> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections")
I think I'll just revert that for v5.9 ?
cheers
> Link: https://lore.kernel.org/lkml/CAKwvOdnn3wxYdJomvnveyD_njwRku3fABWT_bS92duihhywLJQ@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> arch/powerpc/include/asm/vdso.h | 17 ++---------------
> arch/powerpc/kernel/vdso64/Makefile | 8 ++++++--
> arch/powerpc/kernel/vdso64/vdso64.lds.S | 1 -
> 3 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
> index 2ff884853f97..11b2ecf49f79 100644
> --- a/arch/powerpc/include/asm/vdso.h
> +++ b/arch/powerpc/include/asm/vdso.h
> @@ -24,19 +24,7 @@ int vdso_getcpu_init(void);
>
> #else /* __ASSEMBLY__ */
>
> -#ifdef __VDSO64__
> -#define V_FUNCTION_BEGIN(name) \
> - .globl name; \
> - name: \
> -
> -#define V_FUNCTION_END(name) \
> - .size name,.-name;
> -
> -#define V_LOCAL_FUNC(name) (name)
> -#endif /* __VDSO64__ */
> -
> -#ifdef __VDSO32__
> -
> +#if defined(__VDSO32__) || defined (__VDSO64__)
> #define V_FUNCTION_BEGIN(name) \
> .globl name; \
> .type name,@function; \
> @@ -46,8 +34,7 @@ int vdso_getcpu_init(void);
> .size name,.-name;
>
> #define V_LOCAL_FUNC(name) (name)
> -
> -#endif /* __VDSO32__ */
> +#endif /* __VDSO{32|64}__ */
>
> #endif /* __ASSEMBLY__ */
>
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 38c317f25141..7ea3ce537d0a 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -32,9 +32,13 @@ $(obj)/%.so: OBJCOPYFLAGS := -S
> $(obj)/%.so: $(obj)/%.so.dbg FORCE
> $(call if_changed,objcopy)
>
> +ldflags-y := -shared -soname linux-vdso64.so.1 \
> + $(call ld-option, --eh-frame-hdr) \
> + $(call ld-option, --orphan-handling=warn) -T
> +
> # actual build commands
> -quiet_cmd_vdso64ld = VDSO64L $@
> - cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn)
> +quiet_cmd_vdso64ld = LD $@
> + cmd_vdso64ld = $(cmd_ld)
>
> # install commands for the unstripped file
> quiet_cmd_vdso_install = INSTALL $@
> diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
> index 4e3a8d4ee614..58c33b704b6a 100644
> --- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
> +++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
> @@ -11,7 +11,6 @@ OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
> OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
> #endif
> OUTPUT_ARCH(powerpc:common64)
> -ENTRY(_start)
>
> SECTIONS
> {
> --
> 2.28.0.402.g5ffc5be6b7-goog
^ permalink raw reply
* Re: [PATCH v4 02/13] powerpc/mm: Move setting pte specific flags to pfn_pte
From: Christophe Leroy @ 2020-09-02 12:30 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200902114222.181353-3-aneesh.kumar@linux.ibm.com>
Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
> powerpc used to set the pte specific flags in set_pte_at(). This is
> different from other architectures. To be consistent with other
> architecture update pfn_pte to set _PAGE_PTE on ppc64. Also, drop now
> unused pte_mkpte.
>
> We add a VM_WARN_ON() to catch the usage of calling set_pte_at()
> without setting _PAGE_PTE bit. We will remove that after a few releases.
>
> With respect to huge pmd entries, pmd_mkhuge() takes care of adding the
> _PAGE_PTE bit.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Small nit below.
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 15 +++++++++------
> arch/powerpc/include/asm/nohash/pgtable.h | 5 -----
> arch/powerpc/mm/pgtable.c | 5 -----
> 3 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 079211968987..2382fd516f6b 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -823,6 +818,14 @@ static inline int pte_none(pte_t pte)
> static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep, pte_t pte, int percpu)
> {
> +
> + VM_WARN_ON(!(pte_raw(pte) & cpu_to_be64(_PAGE_PTE)));
> + /*
> + * Keep the _PAGE_PTE added till we are sure we handle _PAGE_PTE
> + * in all the callers.
> + */
> + pte = __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_PTE));
Wrong alignment, there is a leading space.
> +
> if (radix_enabled())
> return radix__set_pte_at(mm, addr, ptep, pte, percpu);
> return hash__set_pte_at(mm, addr, ptep, pte, percpu);
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-02 12:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Kees Cook, x86, linuxppc-dev, linux-kernel, Al Viro,
linux-fsdevel, Linus Torvalds, Christoph Hellwig
In-Reply-To: <8974838a-a0b1-1806-4a3a-e983deda67ca@csgroup.eu>
On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
>> - return 0;
>> - return (size == 0 || size - 1 <= seg.seg - addr);
>> + if (addr >= TASK_SIZE_MAX)
>> + return false;
>> + if (size == 0)
>> + return false;
>
> __access_ok() was returning true when size == 0 up to now. Any reason to
> return false now ?
No, this is accidental and broken. Can you re-run your benchmark with
this fixed?
^ permalink raw reply
* Re: [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Christophe Leroy @ 2020-09-02 12:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <20200902114222.181353-5-aneesh.kumar@linux.ibm.com>
Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
> ppc64 supports huge vmap only with radix translation. Hence use arch helper
> to determine the huge vmap support.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/debug_vm_pgtable.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 00649b47f6e0..4c73e63b4ceb 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -28,6 +28,7 @@
> #include <linux/swapops.h>
> #include <linux/start_kernel.h>
> #include <linux/sched/mm.h>
> +#include <linux/io.h>
> #include <asm/pgalloc.h>
> #include <asm/tlbflush.h>
>
> @@ -206,11 +207,12 @@ 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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pmd_supported())
What about moving ioremap_pmd_enabled() from mm/ioremap.c to some .h,
and using it ?
As ioremap_pmd_enabled() is defined at all time, no need of #ifdef
> return;
>
> pr_debug("Validating PMD huge\n");
> @@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot)
> 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)
> {
> @@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
> 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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> + if (!arch_ioremap_pud_supported())
What about moving ioremap_pud_enabled() from mm/ioremap.c to some .h,
and using it ?
As ioremap_pud_enabled() is defined at all time, no need of #ifdef
> return;
>
> pr_debug("Validating PUD huge\n");
> @@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot)
> 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(unsigned long pfn, pgprot_t prot) { }
> static void __init pud_advanced_tests(struct mm_struct *mm,
>
Christophe
^ permalink raw reply
* Re: [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support.
From: Aneesh Kumar K.V @ 2020-09-02 12:53 UTC (permalink / raw)
To: Christophe Leroy, linux-mm, akpm; +Cc: linuxppc-dev, Anshuman Khandual
In-Reply-To: <7e88e090-e3af-b26c-ca65-3d98e8f81eb3@csgroup.eu>
On 9/2/20 6:10 PM, Christophe Leroy wrote:
>
>
> Le 02/09/2020 à 13:42, Aneesh Kumar K.V a écrit :
>> ppc64 supports huge vmap only with radix translation. Hence use arch
>> helper
>> to determine the huge vmap support.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> mm/debug_vm_pgtable.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>> index 00649b47f6e0..4c73e63b4ceb 100644
>> --- a/mm/debug_vm_pgtable.c
>> +++ b/mm/debug_vm_pgtable.c
>> @@ -28,6 +28,7 @@
>> #include <linux/swapops.h>
>> #include <linux/start_kernel.h>
>> #include <linux/sched/mm.h>
>> +#include <linux/io.h>
>> #include <asm/pgalloc.h>
>> #include <asm/tlbflush.h>
>> @@ -206,11 +207,12 @@ 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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
>> + if (!arch_ioremap_pmd_supported())
>
> What about moving ioremap_pmd_enabled() from mm/ioremap.c to some .h,
> and using it ?
> As ioremap_pmd_enabled() is defined at all time, no need of #ifdef
>
yes. This was discussed earlier too. IMHO we should do that outside this
series. I guess figuring out ioremap_pmd/pud support can definitely be
simplified. With a generic version like
#ifndef arch_ioremap_pmd_supported
static inline bool arch_ioremap_pmd_supported(void)
{
return false;
}
#endif
>> return;
>> pr_debug("Validating PMD huge\n");
>> @@ -224,6 +226,9 @@ static void __init pmd_huge_tests(pmd_t *pmdp,
>> unsigned long pfn, pgprot_t prot)
>> 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)
>> {
>> @@ -320,11 +325,12 @@ static void __init pud_leaf_tests(unsigned long
>> pfn, pgprot_t prot)
>> 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 (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
>> + if (!arch_ioremap_pud_supported())
>
> What about moving ioremap_pud_enabled() from mm/ioremap.c to some .h,
> and using it ?
> As ioremap_pud_enabled() is defined at all time, no need of #ifdef
>
>> return;
>> pr_debug("Validating PUD huge\n");
>> @@ -338,6 +344,10 @@ static void __init pud_huge_tests(pud_t *pudp,
>> unsigned long pfn, pgprot_t prot)
>> 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(unsigned long pfn, pgprot_t prot)
>> { }
>> static void __init pud_advanced_tests(struct mm_struct *mm,
>>
>
> Christophe
^ permalink raw reply
* Re: [PATCH v3 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
From: Anshuman Khandual @ 2020-09-02 13:05 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: linux-arch, linux-s390, Christophe Leroy, x86, Mike Rapoport,
Qian Cai, Gerald Schaefer, Vineet Gupta, linux-snps-arc,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <68f90b44-b830-58be-3c21-424fee05da37@linux.ibm.com>
On 09/01/2020 12:00 PM, Aneesh Kumar K.V wrote:
> On 9/1/20 9:33 AM, Anshuman Khandual wrote:
>>
>>
>> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>>> 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.
>>
>> Would really like this to get resolved in an uniform and better way
>> instead, i.e a modified hugetlb_advanced_tests() which works on all
>> platforms including ppc64.
>>
>> In absence of a modified version, I do realize the situation here,
>> where DEBUG_VM_PGTABLE test either runs on ppc64 or just completely
>> remove hugetlb_advanced_tests() from other platforms as well.
>>
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>> mm/debug_vm_pgtable.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index a188b6e4e37e..21329c7d672f 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -813,6 +813,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
>>> #endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
>>> }
>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> struct vm_area_struct *vma,
>>> pte_t *ptep, unsigned long pfn,
>>> @@ -855,6 +856,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> pte = huge_ptep_get(ptep);
>>> WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
>>> }
>>> +#endif
>>
>> In the worst case if we could not get a new hugetlb_advanced_tests() test
>> that works on all platforms, this might be the last fallback option. In
>> which case, it will require a proper comment section with a "FIXME: ",
>> explaining the current situation (and that #ifdef is temporary in nature)
>> and a hugetlb_advanced_tests() stub when CONFIG_PPC_BOOK3S_64 is enabled.
>>
>>> #else /* !CONFIG_HUGETLB_PAGE */
>>> static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> static void __init hugetlb_advanced_tests(struct mm_struct *mm,
>>> @@ -1065,7 +1067,9 @@ static int __init debug_vm_pgtable(void)
>>> pud_populate_tests(mm, pudp, saved_pmdp);
>>> spin_unlock(ptl);
>>> +#ifndef CONFIG_PPC_BOOK3S_64
>>> hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
>>> +#endif
>>
>
> I actually wanted to add #ifdef BROKEN. That test is completely broken. Infact I would suggest to remove that test completely.
>
>
>
>> #ifdef will not be required here as there would be a stub definition
>> for hugetlb_advanced_tests() when CONFIG_PPC_BOOK3S_64 is enabled.
>>
>>> spin_lock(&mm->page_table_lock);
>>> p4d_clear_tests(mm, p4dp);
>>>
>>
>> But again, we should really try and avoid taking this path.
>>
>
> To be frank i am kind of frustrated with how this patch series is being looked at. We pushed a completely broken test to upstream and right now we have a code in upstream that crash when booted on ppc64. My attempt has been to make progress here and you definitely seems to be not in agreement to that.
>
I am afraid, this does not accurately represent the situation.
- The second set patch series got merged in it's V5 after accommodating almost
all reviews and objections during previous discussion cycles. For a complete
development log, please refer https://patchwork.kernel.org/cover/11658627/.
- The series has been repeatedly tested on arm64 and x86 platforms for multiple
configurations but build tested on all other enabled platforms. I have always
been dependent on voluntary help from folks on the list to get this tested on
other enabled platforms as I dont have access to such systems. Always assumed
that is the way to go for anything which runs on multiple platforms. So, am I
expected to test on platforms that I dont have access to ? But I am ready to
be corrected here, if the community protocol is not what I have always assumed
it to be.
- Each and every version of the series had appropriately copied all the enabled
platform's mailing list. Also, I had explicitly asked for volunteers to test
this out on platforms apart from x86 and arm64. We had positive response from
all platforms i.e arc, s390, ppc32 but except for ppc64.
https://patchwork.kernel.org/cover/11644771/
https://patchwork.kernel.org/cover/11603713/
- The development cycle provided sufficient time window for any detailed review
and test. I have always been willing to address almost all the issues brought
forward during these discussions. From past experience on this test, there is
an inherent need to understand platform specific details while trying to come
up with something generic enough that works on all platforms. It necessitates
participation from relevant folks to enable this test on a given platform. We
were able to enable this on arm64, x86, arc, s390, powerpc following a similar
model.
- I have to disagree here that the concerned test i.e hugetlb_advanced_tests()
is completely broken. As mentioned before, the idea here has always been to
emulate enough MM objects, so that a given page table helper could be tested.
hugetlb_advanced_tests() seems to be insufficient on ppc64 platform causing it
to crash, which is not the case on other platforms. But it is not perfect and
can be improved upon. Given the constraints i.e limited emulation of objects,
the test tries to do the right thing. Calling it broken is not an appropriate
description.
> At this point I am tempted to suggest we remove the DEBUG_VM_PGTABLE support on ppc64 because AFAIU it doesn't add any value.
Now that this has been proposed [1], along with it any possible urgency factor
out of the way, we could review the proposal with a more long term view making
sure that it improves existing tests and benefits all enabled platforms. Will
look forward to it.
[1] https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200901094423.100149-1-aneesh.kumar@linux.ibm.com/
- Anshuman
^ permalink raw reply
* RE: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: David Laight @ 2020-09-02 13:13 UTC (permalink / raw)
To: 'Christoph Hellwig', Christophe Leroy
Cc: linux-arch@vger.kernel.org, Kees Cook, x86@kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Al Viro, linux-fsdevel@vger.kernel.org, Linus Torvalds
In-Reply-To: <20200902123646.GA31184@lst.de>
From: Christoph Hellwig
> Sent: 02 September 2020 13:37
>
> On Wed, Sep 02, 2020 at 08:15:12AM +0200, Christophe Leroy wrote:
> >> - return 0;
> >> - return (size == 0 || size - 1 <= seg.seg - addr);
> >> + if (addr >= TASK_SIZE_MAX)
> >> + return false;
> >> + if (size == 0)
> >> + return false;
> >
> > __access_ok() was returning true when size == 0 up to now. Any reason to
> > return false now ?
>
> No, this is accidental and broken. Can you re-run your benchmark with
> this fixed?
Is TASK_SIZE_MASK defined such that you can do:
return (addr | size) < TASK_SIZE_MAX) || !size;
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox