2023-08-12 22:46:16

by Zach O'Keefe

[permalink] [raw]
Subject: [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()"

The 6.0 commits:

commit 9fec51689ff6 ("mm: thp: kill transparent_hugepage_active()")
commit 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()")

merged "can we have THPs in this VMA?" logic that was previously done
separately by fault-path, khugepaged, and smaps "THPeligible".

During the process, the check on VM_NO_KHUGEPAGED from the khugepaged
path was accidentally added to fault and smaps paths. Certainly the
previous behavior for fault should be restored, and since smaps should
report the union of THP eligibility for fault and khugepaged, also opt
smaps out of this constraint.

Fixes: 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()")
Reported-by: Saurabh Singh Sengar <[email protected]>
Signed-off-by: Zach O'Keefe <[email protected]>
Cc: Yang Shi <[email protected]>
---
mm/huge_memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index eb3678360b97..e098c26d5e2e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -96,11 +96,11 @@ bool hugepage_vma_check(struct vm_area_struct *vma, unsigned long vm_flags,
return in_pf;

/*
- * Special VMA and hugetlb VMA.
+ * khugepaged check for special VMA and hugetlb VMA.
* Must be checked after dax since some dax mappings may have
* VM_MIXEDMAP set.
*/
- if (vm_flags & VM_NO_KHUGEPAGED)
+ if (!in_pf && !smaps && (vm_flags & VM_NO_KHUGEPAGED))
return false;

/*
--
2.41.0.694.ge786442a9b-goog



2023-08-13 07:10:33

by Saurabh Singh Sengar

[permalink] [raw]
Subject: RE: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()"



> -----Original Message-----
> From: Zach O'Keefe <[email protected]>
> Sent: Sunday, August 13, 2023 2:31 AM
> To: [email protected]; Yang Shi <[email protected]>
> Cc: [email protected]; Zach O'Keefe <[email protected]>;
> Saurabh Singh Sengar <[email protected]>
> Subject: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill
> __transhuge_page_enabled()"
>
> [You don't often get email from [email protected]. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The 6.0 commits:
>
> commit 9fec51689ff6 ("mm: thp: kill transparent_hugepage_active()") commit
> 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()")
>
> merged "can we have THPs in this VMA?" logic that was previously done
> separately by fault-path, khugepaged, and smaps "THPeligible".
>
> During the process, the check on VM_NO_KHUGEPAGED from the
> khugepaged path was accidentally added to fault and smaps paths. Certainly
> the previous behavior for fault should be restored, and since smaps should
> report the union of THP eligibility for fault and khugepaged, also opt smaps
> out of this constraint.
>
> Fixes: 7da4e2cb8b1f ("mm: thp: kill __transhuge_page_enabled()")
> Reported-by: Saurabh Singh Sengar <[email protected]>
> Signed-off-by: Zach O'Keefe <[email protected]>
> Cc: Yang Shi <[email protected]>
> ---
> mm/huge_memory.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c index
> eb3678360b97..e098c26d5e2e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -96,11 +96,11 @@ bool hugepage_vma_check(struct vm_area_struct
> *vma, unsigned long vm_flags,
> return in_pf;
>
> /*
> - * Special VMA and hugetlb VMA.
> + * khugepaged check for special VMA and hugetlb VMA.
> * Must be checked after dax since some dax mappings may have
> * VM_MIXEDMAP set.
> */
> - if (vm_flags & VM_NO_KHUGEPAGED)
> + if (!in_pf && !smaps && (vm_flags & VM_NO_KHUGEPAGED))
> return false;
>
> /*
> --
> 2.41.0.694.ge786442a9b-goog

Thanks for the patch, I realized with the commit 9fec51689ff60,
!vma_is_anonymous restriction is also introduced. To make fault path
work same as before we need relaxation for this check as well. Can we
add the below as will in this patch:

- if (!vma_is_anonymous(vma))
+ if (!is_pf && !vma_is_anonymous(vma))
return false;

- Saurabh