* [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb @ 2026-05-13 2:48 Ye Liu 2026-05-13 3:36 ` Oscar Salvador 0 siblings, 1 reply; 4+ messages in thread From: Ye Liu @ 2026-05-13 2:48 UTC (permalink / raw) To: Miaohe Lin, Andrew Morton; +Cc: Ye Liu, Naoya Horiguchi, linux-mm, linux-kernel 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb 2026-05-13 2:48 [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb Ye Liu @ 2026-05-13 3:36 ` Oscar Salvador 2026-05-13 6:50 ` Ye Liu 0 siblings, 1 reply; 4+ messages in thread From: Oscar Salvador @ 2026-05-13 3:36 UTC (permalink / raw) To: Ye Liu Cc: Miaohe Lin, Andrew Morton, Ye Liu, Naoya Horiguchi, linux-mm, linux-kernel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb 2026-05-13 3:36 ` Oscar Salvador @ 2026-05-13 6:50 ` Ye Liu 2026-05-13 8:38 ` Oscar Salvador 0 siblings, 1 reply; 4+ messages in thread From: Ye Liu @ 2026-05-13 6:50 UTC (permalink / raw) To: Oscar Salvador Cc: Miaohe Lin, Andrew Morton, Ye Liu, Naoya Horiguchi, linux-mm, linux-kernel 在 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb 2026-05-13 6:50 ` Ye Liu @ 2026-05-13 8:38 ` Oscar Salvador 0 siblings, 0 replies; 4+ messages in thread From: Oscar Salvador @ 2026-05-13 8:38 UTC (permalink / raw) To: Ye Liu Cc: Miaohe Lin, Andrew Morton, Ye Liu, Naoya Horiguchi, linux-mm, linux-kernel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-13 8:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-13 2:48 [RESEND PATCH] mm/memory_failure: use bool for hugetlb indicator in try_memory_failure_hugetlb Ye Liu 2026-05-13 3:36 ` Oscar Salvador 2026-05-13 6:50 ` Ye Liu 2026-05-13 8:38 ` Oscar Salvador
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox