From: Anshuman Khandual <anshuman.khandual@arm.com> To: Mark Rutland <mark.rutland@arm.com>, David Hildenbrand <david@redhat.com> Cc: Liu Shixin <liushixin2@huawei.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Denys Vlasenko <dvlasenk@redhat.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, David Hildenbrand <dhildenb@redhat.com>, Rafael Aquini <raquini@redhat.com>, Pasha Tatashin <pasha.tatashin@soleen.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud Date: Thu, 17 Nov 2022 09:54:06 +0530 [thread overview] Message-ID: <53c0c955-7afe-905b-468a-cd7ac81238c5@arm.com> (raw) In-Reply-To: <Y3UF87Z/a7UVThVF@FVFF77S0Q05N.cambridge.arm.com> On 11/16/22 21:16, Mark Rutland wrote: > On Wed, Nov 16, 2022 at 10:08:27AM +0100, David Hildenbrand wrote: >> On 16.11.22 09:38, Liu Shixin wrote: >>> The page table check trigger BUG_ON() unexpectedly when split hugepage: >>> >>> ------------[ cut here ]------------ >>> kernel BUG at mm/page_table_check.c:119! >>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP >>> Dumping ftrace buffer: >>> (ftrace buffer empty) >>> Modules linked in: >>> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748 >>> Hardware name: linux,dummy-virt (DT) >>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) >>> pc : page_table_check_set.isra.0+0x398/0x468 >>> lr : page_table_check_set.isra.0+0x1c0/0x468 >>> [...] >>> Call trace: >>> page_table_check_set.isra.0+0x398/0x468 >>> __page_table_check_pte_set+0x160/0x1c0 >>> __split_huge_pmd_locked+0x900/0x1648 >>> __split_huge_pmd+0x28c/0x3b8 >>> unmap_page_range+0x428/0x858 >>> unmap_single_vma+0xf4/0x1c8 >>> zap_page_range+0x2b0/0x410 >>> madvise_vma_behavior+0xc44/0xe78 >>> do_madvise+0x280/0x698 >>> __arm64_sys_madvise+0x90/0xe8 >>> invoke_syscall.constprop.0+0xdc/0x1d8 >>> do_el0_svc+0xf4/0x3f8 >>> el0_svc+0x58/0x120 >>> el0t_64_sync_handler+0xb8/0xc0 >>> el0t_64_sync+0x19c/0x1a0 >>> [...] >>> >>> On arm64, pmd_present() will return true even if the pmd is invalid. >> >> I assume that's because of the pmd_present_invalid() check. >> >> ... I wonder why that behavior was chosen. Sounds error-prone to me. > > That seems to be down to commit: > > b65399f6111b03df ("arm64/mm: Change THP helpers to comply with generic MM semantics") > > ... apparently because Andrea Arcangelli said this was necessary in: > > https://lore.kernel.org/lkml/20181017020930.GN30832@redhat.com/ > > ... but that does see to contradict what's said in: > > Documentation/mm/arch_pgtable_helpers.rst > > ... which just says: > > pmd_present Tests a valid mapped PMD It should be as follows instead, will update. Not sure about PUD level though, where anon THP is not supported (AFAIK). +---------------------------+--------------------------------------------------+ | pmd_present | Tests if pmd_page() points to valid memory page | +---------------------------+--------------------------------------------------+ > > ... and it's not clear to me why this *only* applies to the PMD level. > > Anshuman? Because THP is supported at PMD level. As Andrea had explained earlier, pmd_present() should return positive if pmd_page() on the entry points to valid memory irrespective of whether the entry is valid/mapped or not. That is the semantics expected in generic THP during PMD split, collapse, migration etc and other memory code walking past such PMD entries. That was my understanding.
WARNING: multiple messages have this Message-ID (diff)
From: Anshuman Khandual <anshuman.khandual@arm.com> To: Mark Rutland <mark.rutland@arm.com>, David Hildenbrand <david@redhat.com> Cc: Liu Shixin <liushixin2@huawei.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Denys Vlasenko <dvlasenk@redhat.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, David Hildenbrand <dhildenb@redhat.com>, Rafael Aquini <raquini@redhat.com>, Pasha Tatashin <pasha.tatashin@soleen.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud Date: Thu, 17 Nov 2022 09:54:06 +0530 [thread overview] Message-ID: <53c0c955-7afe-905b-468a-cd7ac81238c5@arm.com> (raw) In-Reply-To: <Y3UF87Z/a7UVThVF@FVFF77S0Q05N.cambridge.arm.com> On 11/16/22 21:16, Mark Rutland wrote: > On Wed, Nov 16, 2022 at 10:08:27AM +0100, David Hildenbrand wrote: >> On 16.11.22 09:38, Liu Shixin wrote: >>> The page table check trigger BUG_ON() unexpectedly when split hugepage: >>> >>> ------------[ cut here ]------------ >>> kernel BUG at mm/page_table_check.c:119! >>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP >>> Dumping ftrace buffer: >>> (ftrace buffer empty) >>> Modules linked in: >>> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748 >>> Hardware name: linux,dummy-virt (DT) >>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) >>> pc : page_table_check_set.isra.0+0x398/0x468 >>> lr : page_table_check_set.isra.0+0x1c0/0x468 >>> [...] >>> Call trace: >>> page_table_check_set.isra.0+0x398/0x468 >>> __page_table_check_pte_set+0x160/0x1c0 >>> __split_huge_pmd_locked+0x900/0x1648 >>> __split_huge_pmd+0x28c/0x3b8 >>> unmap_page_range+0x428/0x858 >>> unmap_single_vma+0xf4/0x1c8 >>> zap_page_range+0x2b0/0x410 >>> madvise_vma_behavior+0xc44/0xe78 >>> do_madvise+0x280/0x698 >>> __arm64_sys_madvise+0x90/0xe8 >>> invoke_syscall.constprop.0+0xdc/0x1d8 >>> do_el0_svc+0xf4/0x3f8 >>> el0_svc+0x58/0x120 >>> el0t_64_sync_handler+0xb8/0xc0 >>> el0t_64_sync+0x19c/0x1a0 >>> [...] >>> >>> On arm64, pmd_present() will return true even if the pmd is invalid. >> >> I assume that's because of the pmd_present_invalid() check. >> >> ... I wonder why that behavior was chosen. Sounds error-prone to me. > > That seems to be down to commit: > > b65399f6111b03df ("arm64/mm: Change THP helpers to comply with generic MM semantics") > > ... apparently because Andrea Arcangelli said this was necessary in: > > https://lore.kernel.org/lkml/20181017020930.GN30832@redhat.com/ > > ... but that does see to contradict what's said in: > > Documentation/mm/arch_pgtable_helpers.rst > > ... which just says: > > pmd_present Tests a valid mapped PMD It should be as follows instead, will update. Not sure about PUD level though, where anon THP is not supported (AFAIK). +---------------------------+--------------------------------------------------+ | pmd_present | Tests if pmd_page() points to valid memory page | +---------------------------+--------------------------------------------------+ > > ... and it's not clear to me why this *only* applies to the PMD level. > > Anshuman? Because THP is supported at PMD level. As Andrea had explained earlier, pmd_present() should return positive if pmd_page() on the entry points to valid memory irrespective of whether the entry is valid/mapped or not. That is the semantics expected in generic THP during PMD split, collapse, migration etc and other memory code walking past such PMD entries. That was my understanding. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-11-17 4:24 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-16 8:38 [PATCH 0/2] arm64: fix two bug about page table check Liu Shixin 2022-11-16 8:38 ` Liu Shixin 2022-11-16 8:38 ` [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud Liu Shixin 2022-11-16 8:38 ` Liu Shixin 2022-11-16 9:04 ` David Hildenbrand 2022-11-16 9:04 ` David Hildenbrand 2022-11-16 14:59 ` Pasha Tatashin 2022-11-16 14:59 ` Pasha Tatashin 2022-11-17 4:09 ` Anshuman Khandual 2022-11-17 4:09 ` Anshuman Khandual 2022-11-17 6:59 ` Liu Shixin 2022-11-17 6:59 ` Liu Shixin 2022-11-21 15:57 ` Denys Vlasenko 2022-11-21 15:57 ` Denys Vlasenko 2022-11-16 8:38 ` [PATCH 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud Liu Shixin 2022-11-16 8:38 ` Liu Shixin 2022-11-16 9:08 ` David Hildenbrand 2022-11-16 9:08 ` David Hildenbrand 2022-11-16 15:46 ` Mark Rutland 2022-11-16 15:46 ` Mark Rutland 2022-11-17 4:24 ` Anshuman Khandual [this message] 2022-11-17 4:24 ` Anshuman Khandual 2022-11-16 15:18 ` Pasha Tatashin 2022-11-16 15:18 ` Pasha Tatashin 2022-11-16 15:52 ` Mark Rutland 2022-11-16 15:52 ` Mark Rutland 2022-11-17 3:15 ` Liu Shixin 2022-11-17 3:15 ` Liu Shixin
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=53c0c955-7afe-905b-468a-cd7ac81238c5@arm.com \ --to=anshuman.khandual@arm.com \ --cc=catalin.marinas@arm.com \ --cc=david@redhat.com \ --cc=dhildenb@redhat.com \ --cc=dvlasenk@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=liushixin2@huawei.com \ --cc=mark.rutland@arm.com \ --cc=pasha.tatashin@soleen.com \ --cc=raquini@redhat.com \ --cc=wangkefeng.wang@huawei.com \ --cc=will@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.