On Sat, Jul 15, 2023 at 11:17:27AM +0800, Miaohe Lin wrote:
> If unpoison_memory() fails to clear page hwpoisoned flag, return value
> ret is expected to be -EBUSY. But when get_hwpoison_page() returns 1
> and fails to clear page hwpoisoned flag due to races, return value will
> be unexpected 1 leading to users being confused.
Thank you for fixing this.
>
> Fixes: bf181c582588 ("mm/hwpoison: fix unpoison_memory()")
> Signed-off-by: Miaohe Lin <[email protected]>
> ---
> mm/memory-failure.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 9ab97016877e..ac074f82f5b3 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2546,11 +2546,11 @@ int unpoison_memory(unsigned long pfn)
> unpoison_pr_info("Unpoison: failed to grab page %#lx\n",
> pfn, &unpoison_rs);
> } else {
> + ret = -EBUSY;
It seems be a code smell to me that the variable "ret" is used not only to
save the return value of unpoison_memory(), but also to save the return
value from get_hwpoison_page(). So I think that it might be better to use
another auto-variable solely to save the return value of get_hwpoison_page.
Then ret has the initial value -EBUSY at this point and no need to
reinitialize it.
Thanks,
Naoya Horiguchi
> if (PageHuge(p)) {
> huge = true;
> count = folio_free_raw_hwp(folio, false);
> if (count == 0) {
> - ret = -EBUSY;
> folio_put(folio);
> goto unlock_mutex;
> }
> --
> 2.33.0
>
>
>