2022-11-16 08:21:15

by Liu Shixin

[permalink] [raw]
Subject: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud

The page table check trigger BUG_ON() unexpectedly when collapse hugepage:

------------[ cut here ]------------
kernel BUG at mm/page_table_check.c:82!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : page_table_check_clear.isra.0+0x258/0x3f0
lr : page_table_check_clear.isra.0+0x240/0x3f0
[...]
Call trace:
page_table_check_clear.isra.0+0x258/0x3f0
__page_table_check_pmd_clear+0xbc/0x108
pmdp_collapse_flush+0xb0/0x160
collapse_huge_page+0xa08/0x1080
hpage_collapse_scan_pmd+0xf30/0x1590
khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
khugepaged+0x338/0x518
kthread+0x278/0x2f8
ret_from_fork+0x10/0x20
[...]

Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
and so trigger BUG_ON() unexpectedly.

Fix this problem by using pmd_leaf() insteal of pmd_present() in
pmd_user_accessible_page(). Moreover, use pud_leaf() for
pud_user_accessible_page() too.

Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <[email protected]>
Signed-off-by: Liu Shixin <[email protected]>
---
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 71a1af42f0e8..edf6625ce965 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)

static inline bool pmd_user_accessible_page(pmd_t pmd)
{
- return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
+ return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
}

static inline bool pud_user_accessible_page(pud_t pud)
{
- return pud_present(pud) && pud_user(pud);
+ return pud_leaf(pud) && pud_user(pud);
}
#endif

--
2.25.1



2022-11-16 10:26:29

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud

On 16.11.22 09:38, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <[email protected]>
> Signed-off-by: Liu Shixin <[email protected]>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
> }
> #endif
>

Reviewed-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb


2022-11-16 15:34:14

by Pasha Tatashin

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud

On Wed, Nov 16, 2022 at 2:51 AM Liu Shixin <[email protected]> wrote:
>
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in

s/insteal/instead

> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <[email protected]>
> Signed-off-by: Liu Shixin <[email protected]>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);

Thanks a lot. The x86 variants are already using p*d_leaf() in these functions.

Acked-by: Pasha Tatashin <[email protected]>

2022-11-17 04:41:49

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud



On 11/16/22 14:08, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.

Could you please provide the pmd_val() on the pmd entry, that triggers this
BUG_ON() here ? Only additional thing pmd_leaf() ensures, is that the entry
is not a table one.

#define pmd_leaf(pmd) (pmd_present(pmd) && !pmd_table(pmd))

collapse_huge_page() pmd is non-leaf because it has table bit on ?

>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <[email protected]>
> Signed-off-by: Liu Shixin <[email protected]>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
> }
> #endif
>

2022-11-17 07:20:30

by Liu Shixin

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud



On 2022/11/17 12:09, Anshuman Khandual wrote:
>
> On 11/16/22 14:08, Liu Shixin wrote:
>> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>>
>> ------------[ cut here ]------------
>> kernel BUG at mm/page_table_check.c:82!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
>> Hardware name: linux,dummy-virt (DT)
>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : page_table_check_clear.isra.0+0x258/0x3f0
>> lr : page_table_check_clear.isra.0+0x240/0x3f0
>> [...]
>> Call trace:
>> page_table_check_clear.isra.0+0x258/0x3f0
>> __page_table_check_pmd_clear+0xbc/0x108
>> pmdp_collapse_flush+0xb0/0x160
>> collapse_huge_page+0xa08/0x1080
>> hpage_collapse_scan_pmd+0xf30/0x1590
>> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
>> khugepaged+0x338/0x518
>> kthread+0x278/0x2f8
>> ret_from_fork+0x10/0x20
>> [...]
>>
>> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
>> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
>> and so trigger BUG_ON() unexpectedly.
> Could you please provide the pmd_val() on the pmd entry, that triggers this
> BUG_ON() here ? Only additional thing pmd_leaf() ensures, is that the entry
> is not a table one.
>
> #define pmd_leaf(pmd) (pmd_present(pmd) && !pmd_table(pmd))
>
> collapse_huge_page() pmd is non-leaf because it has table bit on ?
The pmd_val is 0x80000004c367003. It is indeed a table entry. collapse_huge_page()
will replace page table of page granularity with block granularity. Before this replace,
it will call pmdp_collapse_flush() to clear the table pmd. In this function, the table pmd
do the check unexpectdly and trigger the BUG_ON().

diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
index 90ab721a12a8..a5c2380bac4d 100644
--- a/mm/pgtable-generic.c
+++ b/mm/pgtable-generic.c
@@ -221,6 +221,7 @@ pmd_t pmdp_collapse_flush(struct vm_area_struct *vma, unsigned long address,

VM_BUG_ON(address & ~HPAGE_PMD_MASK);
VM_BUG_ON(pmd_trans_huge(*pmdp));
+ pr_err("pmd_val is 0x%lx\n", pmd_val(*pmdp));
pmd = pmdp_huge_get_and_clear(vma->vm_mm, address, pmdp);

/* collapse entails shooting down ptes not pmd */

Thanks,
Liu Shixin
.

>
>> Fix this problem by using pmd_leaf() insteal of pmd_present() in
>> pmd_user_accessible_page(). Moreover, use pud_leaf() for
>> pud_user_accessible_page() too.
>>
>> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
>> Reported-by: Denys Vlasenko <[email protected]>
>> Signed-off-by: Liu Shixin <[email protected]>
>> ---
>> arch/arm64/include/asm/pgtable.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 71a1af42f0e8..edf6625ce965 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>>
>> static inline bool pmd_user_accessible_page(pmd_t pmd)
>> {
>> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>> }
>>
>> static inline bool pud_user_accessible_page(pud_t pud)
>> {
>> - return pud_present(pud) && pud_user(pud);
>> + return pud_leaf(pud) && pud_user(pud);
>> }
>> #endif
>>
> .
>


2022-11-21 16:12:11

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud

On 11/16/22 09:38, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <[email protected]>
> Signed-off-by: Liu Shixin <[email protected]>


Tested on 6.0.6 kernel, no oopses anymore.