From: Gerald Schaefer <email@example.com> To: Anshuman Khandual <firstname.lastname@example.org> Cc: "email@example.com" <firstname.lastname@example.org>, "Aneesh Kumar K.V" <email@example.com>, firstname.lastname@example.org, Vineet Gupta <email@example.com>, firstname.lastname@example.org, "email@example.com" <firstname.lastname@example.org>, email@example.com, linux-riscv <firstname.lastname@example.org>, Gerald Schaefer <email@example.com> Subject: Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes Date: Tue, 8 Sep 2020 17:39:06 +0200 Message-ID: <20200908173906.30fffaa0@thinkpad> (raw) In-Reply-To: <20200904180115.07ee5f00@thinkpad> On Fri, 4 Sep 2020 18:01:15 +0200 Gerald Schaefer <firstname.lastname@example.org> wrote: [...] > > BTW2, a quick test with this change (so far) made the issues on s390 > go away: > > @@ -1069,7 +1074,7 @@ static int __init debug_vm_pgtable(void) > spin_unlock(ptl); > > #ifndef CONFIG_PPC_BOOK3S_64 > - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); > + hugetlb_advanced_tests(mm, vma, (pte_t *) pmdp, pmd_aligned, vaddr, prot); > #endif > > spin_lock(&mm->page_table_lock); > > That would more match the "pte_t pointer" usage for hugetlb code, > i.e. just cast a pmd_t pointer to it. Also changed to pmd_aligned, > but I think the root cause is the pte_t pointer. > > Not entirely sure though if that would really be the correct fix. > I somehow lost whatever little track I had about what these tests > really want to check, and if that would still be valid with that > change. Uh oh, wasn't aware that this (or some predecessor) already went upstream, and broke our debug kernel today. I found out now what goes (horribly) wrong on s390, see below for more details. In short, using hugetlb primitives with ptep pointers that do _not_ point to a pmd or pud entry will not work on s390. It also seems to make no sense to verify / test such a thing in general, as it would also be a severe bug if any kernel code would do that. After all, with hugepages, there are no pte tables, only pmd etc. tables. My change above would fix the issue for s390, but I can still not completely judge if that would not break other things for your tests. In general, for normal kernel code, much of what you do would be very broken, but I guess your tests are doing such "special" things because they can. E.g. because they operate on some "sandbox" mm and page tables, and you also do not need properly populated page tables for some exit / free cleanup, you just throw them away explicitly with pXd_free at the end. So it might just be "the right thing" to pass a casted pmd pointer to hugetlb_advanced_tests(), to simulate and test (proper) usage of the hugetlb primitives. I also see no other way to make this work for s390, than using a proper pmd/pud pointer. If not possible, please add us to the #ifndef. So, for all those interested, here is what goes wrong on s390. huge_ptep_get_and_clear() uses the "idte" instruction for the clearing (and TLB invalidation) part. That instruction expects a "region or segment table" origin, which is a pmd/pud/p4d/pgd, but not a pte table. Even worse, when we calculate the table origin from the given ptep (which *should* not point to a pte), due to different table sizes for pte / pXd tables, we end up at some place before the given pte table. The "idte" instruction also gets the virtual address, and does corresponding index addition to the given table origin. Depending on the pmd_index we now end up either within the pte table again, in which case we see a panic because idte complains about seeing a pte value. If we are unlucky, then we end up outside the pte table, and depending on the content of that memory location, idte might succeed, effectively corrupting that memory. That explains why we only see the panic sometimes, depending on random vaddr, other symptoms other times, and probably completely silent memory corruption for the rest...
next prev parent reply index Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-02 11:42 Aneesh Kumar K.V 2020-09-02 11:42 ` [PATCH v4 01/13] powerpc/mm: Add DEBUG_VM WARN for pmd_clear Aneesh Kumar K.V 2020-09-02 11:42 ` [PATCH v4 02/13] powerpc/mm: Move setting pte specific flags to pfn_pte Aneesh Kumar K.V 2020-09-02 12:30 ` Christophe Leroy 2020-09-02 11:42 ` [PATCH v4 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value Aneesh Kumar K.V 2020-09-04 4:03 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 04/13] mm/debug_vm_pgtables/hugevmap: Use the arch helper to identify huge vmap support Aneesh Kumar K.V 2020-09-02 12:40 ` Christophe Leroy 2020-09-02 12:53 ` Aneesh Kumar K.V 2020-09-04 4:08 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 05/13] mm/debug_vm_pgtable/savedwrite: Enable savedwrite test with CONFIG_NUMA_BALANCING Aneesh Kumar K.V 2020-09-04 4:09 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 06/13] mm/debug_vm_pgtable/THP: Mark the pte entry huge before using set_pmd/pud_at Aneesh Kumar K.V 2020-09-04 5:21 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 07/13] mm/debug_vm_pgtable/set_pte/pmd/pud: Don't use set_*_at to update an existing pte entry Aneesh Kumar K.V 2020-09-04 5:34 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 08/13] mm/debug_vm_pgtable/locks: Move non page table modifying test together Aneesh Kumar K.V 2020-09-04 3:58 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 09/13] mm/debug_vm_pgtable/locks: Take correct page table lock Aneesh Kumar K.V 2020-09-04 5:39 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 10/13] mm/debug_vm_pgtable/thp: Use page table depost/withdraw with THP Aneesh Kumar K.V 2020-09-04 4:21 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 11/13] mm/debug_vm_pgtable/pmd_clear: Don't use pmd/pud_clear on pte entries Aneesh Kumar K.V 2020-09-04 6:03 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 12/13] mm/debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64 Aneesh Kumar K.V 2020-09-04 6:19 ` Anshuman Khandual 2020-09-02 11:42 ` [PATCH v4 13/13] mm/debug_vm_pgtable: Avoid none pte in pte_clear_test Aneesh Kumar K.V 2020-09-11 2:13 ` Nathan Chancellor 2020-09-11 5:21 ` Aneesh Kumar K.V 2020-09-23 3:14 ` Anshuman Khandual 2020-10-11 20:02 ` Guenter Roeck 2020-10-12 4:29 ` Aneesh Kumar K.V 2020-09-02 11:46 ` Aneesh Kumar K.V 2020-09-04 4:16 ` Anshuman Khandual 2020-09-04 6:48 ` [PATCH v4 00/13] mm/debug_vm_pgtable fixes Anshuman Khandual 2020-09-04 15:26 ` Gerald Schaefer 2020-09-04 16:01 ` Gerald Schaefer 2020-09-04 17:53 ` Gerald Schaefer 2020-09-09 8:38 ` Anshuman Khandual 2020-09-08 15:39 ` Gerald Schaefer [this message] 2020-09-09 6:08 ` Aneesh Kumar K.V 2020-09-09 11:16 ` Gerald Schaefer 2020-09-09 8:15 ` Anshuman Khandual 2020-09-09 11:10 ` Gerald Schaefer 2020-09-09 8:08 ` Anshuman Khandual 2020-09-09 11:36 ` Gerald Schaefer 2020-09-13 11:03 ` [PATCH] mm/debug_vm_pgtable: Avoid doing memory allocation with pgtable_t mapped Aneesh Kumar K.V 2020-09-13 13:42 ` Matthew Wilcox 2020-10-13 20:58 ` [PATCH v4 00/13] mm/debug_vm_pgtable fixes Andrew Morton 2020-10-14 3:15 ` Aneesh Kumar K.V 2020-10-14 20:36 ` Andrew Morton 2020-10-15 2:59 ` Anshuman Khandual
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200908173906.30fffaa0@thinkpad \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
LinuxPPC-Dev Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \ firstname.lastname@example.org email@example.com public-inbox-index linuxppc-dev Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git