Re: [PATCH v1 3/5] mm, hwpoison: make __page_handle_poison returns int

From: Miaohe Lin
Date: Tue Jun 07 2022 - 08:54:47 EST


On 2022/6/2 13:06, Naoya Horiguchi wrote:
> From: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
>
> __page_handle_poison() returns bool that shows whether
> take_page_off_buddy() has passed or not now. But we will want to
> distinguish another case of "dissolve has passed but taking off failed"
> by its return value. So change the type of the return value.
> No functional change.
>
> Signed-off-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
> ---
> mm/memory-failure.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index fe6a7961dc66..f149a7864c81 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -68,7 +68,13 @@ int sysctl_memory_failure_recovery __read_mostly = 1;
>
> atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
>
> -static bool __page_handle_poison(struct page *page)
> +/*
> + * Return values:
> + * 1: the page is dissolved (if needed) and taken off from buddy,
> + * 0: the page is dissolved (if needed) and not taken off from buddy,
> + * < 0: failed to dissolve.
> + */
> +static int __page_handle_poison(struct page *page)
> {
> int ret;
>
> @@ -78,7 +84,7 @@ static bool __page_handle_poison(struct page *page)
> ret = take_page_off_buddy(page);
> zone_pcp_enable(page_zone(page));
>
> - return ret > 0;
> + return ret;
> }
>
> static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, bool release)
> @@ -88,7 +94,7 @@ static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, boo
> * Doing this check for free pages is also fine since dissolve_free_huge_page
> * returns 0 for non-hugetlb pages as well.
> */
> - if (!__page_handle_poison(page))
> + if (__page_handle_poison(page) <= 0)
> /*
> * We could fail to take off the target page from buddy
> * for example due to racy page allocation, but that's
> @@ -1045,7 +1051,7 @@ static int me_huge_page(struct page_state *ps, struct page *p)
> * save healthy subpages.
> */
> put_page(hpage);
> - if (__page_handle_poison(p)) {
> + if (__page_handle_poison(p) > 0) {
> page_ref_inc(p);
> res = MF_RECOVERED;
> }
> @@ -1595,8 +1601,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
> */
> if (res == 0) {
> unlock_page(head);
> - res = MF_FAILED;

This looks like an unexpected change. res will be 0 instead of MF_FAILED if __page_handle_poison failed to
dissolve or not taken off from buddy. But this is fixed in later patch in this series. So it should be fine.

Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>

Thanks!

> - if (__page_handle_poison(p)) {
> + if (__page_handle_poison(p) > 0) {
> page_ref_inc(p);
> res = MF_RECOVERED;
> }
>