[RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb

[RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb

[Ye Liu]

From: Ye Liu <liuye@kylinos.cn>

The hugetlb indicator in try_memory_failure_hugetlb is a Boolean
flag, but was declared and assigned as int/0/1. Convert to `bool`
and `true`/`false` for clarity and type safety.

- try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
- testcase path in memory_failure(): bool hugetlb = false
- clear semantic conversion in MF_HUGETLB_NON_HUGEPAGE
- preserve behavior (no functional change)

Signed-off-by: Ye Liu <liuye@kylinos.cn>
Acked-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 866c4428ac7e..f164fc5959f0 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2032,7 +2032,7 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
  *	-EHWPOISON	- folio or exact page already poisoned
  *	-EFAULT		- kill_accessing_process finds current->mm null
  */
-static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
+static int try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
 {
 	int res, rv;
 	struct page *p = pfn_to_page(pfn);
@@ -2040,12 +2040,12 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
 	unsigned long page_flags;
 	bool migratable_cleared = false;
 
-	*hugetlb = 1;
+	*hugetlb = true;
 retry:
 	res = get_huge_page_for_hwpoison(pfn, flags, &migratable_cleared);
 	switch (res) {
 	case MF_HUGETLB_NON_HUGEPAGE:	/* fallback to normal page handling */
-		*hugetlb = 0;
+		*hugetlb = false;
 		return 0;
 	case MF_HUGETLB_RETRY:
 		if (!(flags & MF_NO_RETRY)) {
@@ -2107,7 +2107,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
 }
 
 #else
-static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
+static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
 {
 	return 0;
 }
@@ -2347,7 +2347,7 @@ int memory_failure(unsigned long pfn, int flags)
 	int res = 0;
 	unsigned long page_flags;
 	bool retry = true;
-	int hugetlb = 0;
+	bool hugetlb = false;
 
 	if (!sysctl_memory_failure_recovery)
 		panic("Memory failure on page %lx", pfn);
-- 
2.43.0

Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb

[Oscar Salvador]

On Wed, May 13, 2026 at 10:48:52AM +0800, Ye Liu wrote:
> From: Ye Liu <liuye@kylinos.cn>
> 
> The hugetlb indicator in try_memory_failure_hugetlb is a Boolean
> flag, but was declared and assigned as int/0/1. Convert to `bool`
> and `true`/`false` for clarity and type safety.
> 
> - try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
> - testcase path in memory_failure(): bool hugetlb = false
> - clear semantic conversion in MF_HUGETLB_NON_HUGEPAGE
> - preserve behavior (no functional change)
> 
> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> Acked-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/memory-failure.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 866c4428ac7e..f164fc5959f0 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2032,7 +2032,7 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
>   *	-EHWPOISON	- folio or exact page already poisoned
>   *	-EFAULT		- kill_accessing_process finds current->mm null
>   */
> -static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
> +static int try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
>  {
>  	int res, rv;
>  	struct page *p = pfn_to_page(pfn);
> @@ -2040,12 +2040,12 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
>  	unsigned long page_flags;
>  	bool migratable_cleared = false;
>  
> -	*hugetlb = 1;
> +	*hugetlb = true;
>  retry:
>  	res = get_huge_page_for_hwpoison(pfn, flags, &migratable_cleared);
>  	switch (res) {
>  	case MF_HUGETLB_NON_HUGEPAGE:	/* fallback to normal page handling */
> -		*hugetlb = 0;
> +		*hugetlb = false;

Do we really need this boolean though?
Why do not simply return ENOENT when we find a non-hugetlb page, and then the caller
knows that if we get ENOENT, it can proceed with normal page handling?

I might be missing something, but is not the following more cleaer?

 diff --git a/mm/memory-failure.c b/mm/memory-failure.c
 index ee42d4361309..44f388df5731 100644
 --- a/mm/memory-failure.c
 +++ b/mm/memory-failure.c
 @@ -2026,13 +2026,14 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
   * So some of prechecks for hwpoison (pinning, and testing/setting
   * PageHWPoison) should be done in single hugetlb_lock range.
   * Returns:
 - *     0               - not hugetlb, or recovered
 + *     0               - recovered
 + *     -ENOENT         - no hugetlb page
   *     -EBUSY          - not recovered
   *     -EOPNOTSUPP     - hwpoison_filter'ed
   *     -EHWPOISON      - folio or exact page already poisoned
   *     -EFAULT         - kill_accessing_process finds current->mm null
   */
 -static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
 +static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
  {
         int res, rv;
         struct page *p = pfn_to_page(pfn);
 @@ -2040,13 +2041,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
         unsigned long page_flags;
         bool migratable_cleared = false;
 
 -       *hugetlb = 1;
  retry:
         res = get_huge_page_for_hwpoison(pfn, flags, &migratable_cleared);
         switch (res) {
         case MF_HUGETLB_NON_HUGEPAGE:   /* fallback to normal page handling */
 -               *hugetlb = 0;
 -               return 0;
 +               return -ENOENT;
         case MF_HUGETLB_RETRY:
                 if (!(flags & MF_NO_RETRY)) {
                         flags |= MF_NO_RETRY;
 @@ -2107,7 +2106,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
  }
 
  #else
 -static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
 +static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags)
  {
         return 0;
  }
 @@ -2386,8 +2385,11 @@ int memory_failure(unsigned long pfn, int flags)
         }
 
  try_again:
 -       res = try_memory_failure_hugetlb(pfn, flags, &hugetlb);
 -       if (hugetlb)
 +       res = try_memory_failure_hugetlb(pfn, flags);
 +       /*
 +        * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling
 +        */
 +       if (res != -ENOENT)
                 goto unlock_mutex;
 
         if (TestSetPageHWPoison(p)) {


-- 
Oscar Salvador
SUSE Labs

Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb

[Ye Liu]

在 2026/5/13 11:36, Oscar Salvador 写道:
> On Wed, May 13, 2026 at 10:48:52AM +0800, Ye Liu wrote:
>> From: Ye Liu <liuye@kylinos.cn>
>>
>> The hugetlb indicator in try_memory_failure_hugetlb is a Boolean
>> flag, but was declared and assigned as int/0/1. Convert to `bool`
>> and `true`/`false` for clarity and type safety.
>>
>> - try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
>> - testcase path in memory_failure(): bool hugetlb = false
>> - clear semantic conversion in MF_HUGETLB_NON_HUGEPAGE
>> - preserve behavior (no functional change)
>>
>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>> Acked-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/memory-failure.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index 866c4428ac7e..f164fc5959f0 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -2032,7 +2032,7 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
>>   *	-EHWPOISON	- folio or exact page already poisoned
>>   *	-EFAULT		- kill_accessing_process finds current->mm null
>>   */
>> -static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
>> +static int try_memory_failure_hugetlb(unsigned long pfn, int flags, bool *hugetlb)
>>  {
>>  	int res, rv;
>>  	struct page *p = pfn_to_page(pfn);
>> @@ -2040,12 +2040,12 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
>>  	unsigned long page_flags;
>>  	bool migratable_cleared = false;
>>  
>> -	*hugetlb = 1;
>> +	*hugetlb = true;
>>  retry:
>>  	res = get_huge_page_for_hwpoison(pfn, flags, &migratable_cleared);
>>  	switch (res) {
>>  	case MF_HUGETLB_NON_HUGEPAGE:	/* fallback to normal page handling */
>> -		*hugetlb = 0;
>> +		*hugetlb = false;
> 
> Do we really need this boolean though?
> Why do not simply return ENOENT when we find a non-hugetlb page, and then the caller
> knows that if we get ENOENT, it can proceed with normal page handling?
> 
> I might be missing something, but is not the following more cleaer?
> 
>  diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>  index ee42d4361309..44f388df5731 100644
>  --- a/mm/memory-failure.c
>  +++ b/mm/memory-failure.c
>  @@ -2026,13 +2026,14 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
>    * So some of prechecks for hwpoison (pinning, and testing/setting
>    * PageHWPoison) should be done in single hugetlb_lock range.
>    * Returns:
>  - *     0               - not hugetlb, or recovered
>  + *     0               - recovered
>  + *     -ENOENT         - no hugetlb page
>    *     -EBUSY          - not recovered
>    *     -EOPNOTSUPP     - hwpoison_filter'ed
>    *     -EHWPOISON      - folio or exact page already poisoned
>    *     -EFAULT         - kill_accessing_process finds current->mm null
>    */
>  -static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
>  +static int try_memory_failure_hugetlb(unsigned long pfn, int flags)
>   {
>          int res, rv;
>          struct page *p = pfn_to_page(pfn);
>  @@ -2040,13 +2041,11 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
>          unsigned long page_flags;
>          bool migratable_cleared = false;
>  
>  -       *hugetlb = 1;
>   retry:
>          res = get_huge_page_for_hwpoison(pfn, flags, &migratable_cleared);
>          switch (res) {
>          case MF_HUGETLB_NON_HUGEPAGE:   /* fallback to normal page handling */
>  -               *hugetlb = 0;
>  -               return 0;
>  +               return -ENOENT;
>          case MF_HUGETLB_RETRY:
>                  if (!(flags & MF_NO_RETRY)) {
>                          flags |= MF_NO_RETRY;
>  @@ -2107,7 +2106,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
>   }
>  
>   #else
>  -static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
>  +static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags)
>   {
>          return 0;
>   }
>  @@ -2386,8 +2385,11 @@ int memory_failure(unsigned long pfn, int flags)
>          }
>  
>   try_again:
>  -       res = try_memory_failure_hugetlb(pfn, flags, &hugetlb);
>  -       if (hugetlb)
>  +       res = try_memory_failure_hugetlb(pfn, flags);
>  +       /*
>  +        * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling
>  +        */
>  +       if (res != -ENOENT)
>                  goto unlock_mutex;
>  
>          if (TestSetPageHWPoison(p)) {
> 
> 


Hi Oscar,

Good point.  Using -ENOENT to distinguish "not a hugetlb page" from
"hugetlb handled" is indeed cleaner than carrying an extra output
parameter.

One thing to note: the #else stub when CONFIG_HUGETLB_PAGE is not set
currently does:

    return 0;

which with your change would mean "hugetlb handled, skip normal path"
instead of the intended "not hugetlb, proceed with normal handling".
It should be changed to:

    return -ENOENT;

Otherwise the non-hugetlb config would silently skip all normal page
failure processing.

-- 
Thanks,
Ye Liu

Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb

[Oscar Salvador]

On Wed, May 13, 2026 at 02:50:56PM +0800, Ye Liu wrote:
> >  -static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb)
> >  +static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags)
> >   {
> >          return 0;
> >   }
> >  @@ -2386,8 +2385,11 @@ int memory_failure(unsigned long pfn, int flags)
> >          }
> >  
> >   try_again:
> >  -       res = try_memory_failure_hugetlb(pfn, flags, &hugetlb);
> >  -       if (hugetlb)
> >  +       res = try_memory_failure_hugetlb(pfn, flags);
> >  +       /*
> >  +        * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling
> >  +        */
> >  +       if (res != -ENOENT)
> >                  goto unlock_mutex;
> >  
> >          if (TestSetPageHWPoison(p)) {
> > 
> > 
> 
> 
> Hi Oscar,
> 
> Good point.  Using -ENOENT to distinguish "not a hugetlb page" from
> "hugetlb handled" is indeed cleaner than carrying an extra output
> parameter.
> 
> One thing to note: the #else stub when CONFIG_HUGETLB_PAGE is not set
> currently does:
> 
>     return 0;
> 
> which with your change would mean "hugetlb handled, skip normal path"
> instead of the intended "not hugetlb, proceed with normal handling".
> It should be changed to:
> 
>     return -ENOENT;

Right, let us see if Miaohe sees any issue with that approach.

 

-- 
Oscar Salvador
SUSE Labs