Use enum scan_result for local variables and the result pointer in
khugepaged_scan_mm_slot(), instead of plain int. This improves code
readability and clarifies intent,
No functional change.
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
mm/khugepaged.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 107146f012b1..65b1b778378a 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -547,7 +547,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
struct folio *folio = NULL;
unsigned long addr = start_addr;
pte_t *_pte;
- int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
+ int none_or_zero = 0, shared = 0, referenced = 0;
+ enum scan_result result = SCAN_FAIL;
for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
_pte++, addr += PAGE_SIZE) {
@@ -786,7 +787,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
struct list_head *compound_pagelist)
{
unsigned int i;
- int result = SCAN_SUCCEED;
+ enum scan_result result = SCAN_SUCCEED;
/*
* Copying pages' contents is subject to memory poison at any iteration.
@@ -969,7 +970,7 @@ static int check_pmd_still_valid(struct mm_struct *mm,
pmd_t *pmd)
{
pmd_t *new_pmd;
- int result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
+ enum scan_result result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
if (result != SCAN_SUCCEED)
return result;
@@ -993,7 +994,7 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm,
int swapped_in = 0;
vm_fault_t ret = 0;
unsigned long addr, end = start_addr + (HPAGE_PMD_NR * PAGE_SIZE);
- int result;
+ enum scan_result result;
pte_t *pte = NULL;
spinlock_t *ptl;
@@ -1100,7 +1101,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
pgtable_t pgtable;
struct folio *folio;
spinlock_t *pmd_ptl, *pte_ptl;
- int result = SCAN_FAIL;
+ enum scan_result result = SCAN_FAIL;
struct vm_area_struct *vma;
struct mmu_notifier_range range;
@@ -1253,8 +1254,8 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
{
pmd_t *pmd;
pte_t *pte, *_pte;
- int result = SCAN_FAIL, referenced = 0;
- int none_or_zero = 0, shared = 0;
+ int none_or_zero = 0, shared = 0, referenced = 0;
+ enum scan_result result = SCAN_FAIL;
struct page *page = NULL;
struct folio *folio = NULL;
unsigned long addr;
@@ -1492,7 +1493,8 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr,
int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
bool install_pmd)
{
- int nr_mapped_ptes = 0, result = SCAN_FAIL;
+ enum scan_result result = SCAN_FAIL;
+ int nr_mapped_ptes = 0;
unsigned int nr_batch_ptes;
struct mmu_notifier_range range;
bool notified = false;
@@ -1866,7 +1868,8 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
pgoff_t index = 0, end = start + HPAGE_PMD_NR;
LIST_HEAD(pagelist);
XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
- int nr_none = 0, result = SCAN_SUCCEED;
+ enum scan_result result = SCAN_SUCCEED;
+ int nr_none = 0;
bool is_shmem = shmem_file(file);
VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
@@ -2296,7 +2299,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
XA_STATE(xas, &mapping->i_pages, start);
int present, swap;
int node = NUMA_NO_NODE;
- int result = SCAN_SUCCEED;
+ enum scan_result result = SCAN_SUCCEED;
present = 0;
swap = 0;
@@ -2394,7 +2397,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
return result;
}
-static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
+static unsigned int khugepaged_scan_mm_slot(unsigned int pages, enum scan_result *result,
struct collapse_control *cc)
__releases(&khugepaged_mm_lock)
__acquires(&khugepaged_mm_lock)
@@ -2555,7 +2558,7 @@ static void khugepaged_do_scan(struct collapse_control *cc)
unsigned int progress = 0, pass_through_head = 0;
unsigned int pages = READ_ONCE(khugepaged_pages_to_scan);
bool wait = true;
- int result = SCAN_SUCCEED;
+ enum scan_result result = SCAN_SUCCEED;
lru_add_drain_all();
@@ -2790,7 +2793,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) {
bool retried = false;
- int result = SCAN_FAIL;
+ enum scan_result result = SCAN_FAIL;
if (!mmap_locked) {
retry:
--
2.43.0On 16 Dec 2025, at 6:11, Shivank Garg wrote:
> Use enum scan_result for local variables and the result pointer in
> khugepaged_scan_mm_slot(), instead of plain int. This improves code
> readability and clarifies intent,
>
> No functional change.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> mm/khugepaged.c | 29 ++++++++++++++++-------------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 107146f012b1..65b1b778378a 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -547,7 +547,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
The return type could be changed too.
> struct folio *folio = NULL;
> unsigned long addr = start_addr;
> pte_t *_pte;
> - int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
> + int none_or_zero = 0, shared = 0, referenced = 0;
> + enum scan_result result = SCAN_FAIL;
>
> for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
> _pte++, addr += PAGE_SIZE) {
> @@ -786,7 +787,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
Ditto.
> struct list_head *compound_pagelist)
> {
> unsigned int i;
> - int result = SCAN_SUCCEED;
> + enum scan_result result = SCAN_SUCCEED;
>
> /*
> * Copying pages' contents is subject to memory poison at any iteration.
> @@ -969,7 +970,7 @@ static int check_pmd_still_valid(struct mm_struct *mm,
> pmd_t *pmd)
Ditto.
> {
> pmd_t *new_pmd;
> - int result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
> + enum scan_result result = find_pmd_or_thp_or_none(mm, address, &new_pmd);
>
> if (result != SCAN_SUCCEED)
> return result;
> @@ -993,7 +994,7 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm,
Ditto.
> int swapped_in = 0;
> vm_fault_t ret = 0;
> unsigned long addr, end = start_addr + (HPAGE_PMD_NR * PAGE_SIZE);
> - int result;
> + enum scan_result result;
> pte_t *pte = NULL;
> spinlock_t *ptl;
>
> @@ -1100,7 +1101,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
Ditto.
> pgtable_t pgtable;
> struct folio *folio;
> spinlock_t *pmd_ptl, *pte_ptl;
> - int result = SCAN_FAIL;
> + enum scan_result result = SCAN_FAIL;
> struct vm_area_struct *vma;
> struct mmu_notifier_range range;
>
> @@ -1253,8 +1254,8 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
> {
Ditto.
> pmd_t *pmd;
> pte_t *pte, *_pte;
> - int result = SCAN_FAIL, referenced = 0;
> - int none_or_zero = 0, shared = 0;
> + int none_or_zero = 0, shared = 0, referenced = 0;
> + enum scan_result result = SCAN_FAIL;
> struct page *page = NULL;
> struct folio *folio = NULL;
> unsigned long addr;
> @@ -1492,7 +1493,8 @@ static int set_huge_pmd(struct vm_area_struct *vma, unsigned long addr,
Same here.
> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
> bool install_pmd)
> {
And here.
> - int nr_mapped_ptes = 0, result = SCAN_FAIL;
> + enum scan_result result = SCAN_FAIL;
> + int nr_mapped_ptes = 0;
> unsigned int nr_batch_ptes;
> struct mmu_notifier_range range;
> bool notified = false;
> @@ -1866,7 +1868,8 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
Same here.
> pgoff_t index = 0, end = start + HPAGE_PMD_NR;
> LIST_HEAD(pagelist);
> XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> - int nr_none = 0, result = SCAN_SUCCEED;
> + enum scan_result result = SCAN_SUCCEED;
> + int nr_none = 0;
> bool is_shmem = shmem_file(file);
>
> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> @@ -2296,7 +2299,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
Same here.
> XA_STATE(xas, &mapping->i_pages, start);
> int present, swap;
> int node = NUMA_NO_NODE;
> - int result = SCAN_SUCCEED;
> + enum scan_result result = SCAN_SUCCEED;
>
> present = 0;
> swap = 0;
> @@ -2394,7 +2397,7 @@ static int hpage_collapse_scan_file(struct mm_struct *mm, unsigned long addr,
> return result;
> }
>
> -static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> +static unsigned int khugepaged_scan_mm_slot(unsigned int pages, enum scan_result *result,
> struct collapse_control *cc)
> __releases(&khugepaged_mm_lock)
> __acquires(&khugepaged_mm_lock)
> @@ -2555,7 +2558,7 @@ static void khugepaged_do_scan(struct collapse_control *cc)
> unsigned int progress = 0, pass_through_head = 0;
> unsigned int pages = READ_ONCE(khugepaged_pages_to_scan);
> bool wait = true;
> - int result = SCAN_SUCCEED;
> + enum scan_result result = SCAN_SUCCEED;
>
> lru_add_drain_all();
>
> @@ -2790,7 +2793,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>
> for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) {
> bool retried = false;
> - int result = SCAN_FAIL;
> + enum scan_result result = SCAN_FAIL;
>
> if (!mmap_locked) {
> retry:
> --
In addition, the return types of find_pmd_or_thp_or_none(),
hugepage_vma_revalidate(), alloc_charge_folio(), and check_pmd_state() need to be changed to enum scan_result too.
Best Regards,
Yan, Zi
On 12/16/2025 9:08 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>
>
>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>> bool install_pmd)
>> {
>
> And here.
Since this function is declared in khugepaged.h, I need to
move the enum definition to that header. I see two options for handling
the CONFIG_TRANSPARENT_HUGEPAGE check:
1. Define enum OUTSIDE the ifdef: This allows the static inline stub
to also return enum scan_result, keeping the API consistent.
2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
forcing the stub to return int 0 instead.
The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
ignores the return value.
What is the more preferred way for enum definition in such scenario?
Thanks,
Shivank
> In addition, the return types of find_pmd_or_thp_or_none(),
> hugepage_vma_revalidate(), alloc_charge_folio(), and check_pmd_state() need to be changed to enum scan_result too.
>
> Best Regards,
> Yan, Zi
On 12/18/25 06:40, Garg, Shivank wrote:
>
>
> On 12/16/2025 9:08 PM, Zi Yan wrote:
>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>
>
>>
>>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>> bool install_pmd)
>>> {
>>
>> And here.
> Since this function is declared in khugepaged.h, I need to
> move the enum definition to that header. I see two options for handling
> the CONFIG_TRANSPARENT_HUGEPAGE check:
>
> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
> to also return enum scan_result, keeping the API consistent.
> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
> forcing the stub to return int 0 instead.
>
> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
> ignores the return value.
Probably best to not expose that enum (especially when nobody cares ...)
and instead expose a new void function for uprobe purposes.
Maybe
void collapse_pte_mapped_thp(...)
{
try_collapse_pte_mapped_thp();
}
Maybe something like that?
--
Cheers
David
On 18 Dec 2025, at 4:17, David Hildenbrand (Red Hat) wrote:
> On 12/18/25 06:40, Garg, Shivank wrote:
>>
>>
>> On 12/16/2025 9:08 PM, Zi Yan wrote:
>>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>>
>>
>>>
>>>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>> bool install_pmd)
>>>> {
>>>
>>> And here.
>> Since this function is declared in khugepaged.h, I need to
>> move the enum definition to that header. I see two options for handling
>> the CONFIG_TRANSPARENT_HUGEPAGE check:
>>
>> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>> to also return enum scan_result, keeping the API consistent.
>> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>> forcing the stub to return int 0 instead.
>>
>> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
>> ignores the return value.
>
> Probably best to not expose that enum (especially when nobody cares ...) and instead expose a new void function for uprobe purposes.
>
> Maybe
>
> void collapse_pte_mapped_thp(...)
> {
> try_collapse_pte_mapped_thp();
> }
>
> Maybe something like that?
Sounds good to me.
Best Regards,
Yan, Zi
On 12/18/2025 9:24 PM, Zi Yan wrote:
> On 18 Dec 2025, at 4:17, David Hildenbrand (Red Hat) wrote:
>
>> On 12/18/25 06:40, Garg, Shivank wrote:
>>>
>>>
>>> On 12/16/2025 9:08 PM, Zi Yan wrote:
>>>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>>>
>>>
>>>>
>>>>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>>>> bool install_pmd)
>>>>> {
>>>>
>>>> And here.
>>> Since this function is declared in khugepaged.h, I need to
>>> move the enum definition to that header. I see two options for handling
>>> the CONFIG_TRANSPARENT_HUGEPAGE check:
>>>
>>> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
>>> to also return enum scan_result, keeping the API consistent.
>>> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
>>> forcing the stub to return int 0 instead.
>>>
>>> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
>>> ignores the return value.
>>
>> Probably best to not expose that enum (especially when nobody cares ...) and instead expose a new void function for uprobe purposes.
>>
>> Maybe
>>
>> void collapse_pte_mapped_thp(...)
>> {
>> try_collapse_pte_mapped_thp();
>> }
>>
>> Maybe something like that?
>
> Sounds good to me.
Thanks, this makes sense to me.
I'll do this.
Best Regards,
Shivank
On 12/16/2025 9:08 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>
>> @@ -547,7 +547,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>
> The return type could be changed too.
>
>> struct folio *folio = NULL;
>> unsigned long addr = start_addr;
>> pte_t *_pte;
>> - int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
>> + int none_or_zero = 0, shared = 0, referenced = 0;
>> + enum scan_result result = SCAN_FAIL;
>>
>> for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
>> _pte++, addr += PAGE_SIZE) {
>> @@ -786,7 +787,7 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
>
> Ditto.
>
>> struct list_head *compound_pagelist)
>> {
>> --
>
> In addition, the return types of find_pmd_or_thp_or_none(),
> hugepage_vma_revalidate(), alloc_charge_folio(), and check_pmd_state() need to be changed to enum scan_result too.
>
Hi Zi,
Thanks for the review and suggestions.
I'll update the return types and send V2.
Thanks,
Shivank
© 2016 - 2026 Red Hat, Inc.