Re: [PATCH v1 6/6] mm/hwpoison: fix unpoison_memory()

From: "HORIGUCHI NAOYA(堀口　直也)" <naoya.horiguchi@nec.com>
To: Ding Hui <dinghui@sangfor.com.cn>
Cc: Naoya Horiguchi <nao.horiguchi@gmail.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Oscar Salvador <osalvador@suse.de>,
	Michal Hocko <mhocko@suse.com>, Tony Luck <tony.luck@intel.com>,
	Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 6/6] mm/hwpoison: fix unpoison_memory()
Date: Fri, 18 Jun 2021 08:36:26 +0000	[thread overview]
Message-ID: <20210618083625.GA2215283@hori.linux.bs1.fc.nec.co.jp> (raw)
In-Reply-To: <94984984-f123-85ae-20bc-b40e90d536f6@sangfor.com.cn>

On Thu, Jun 17, 2021 at 06:00:21PM +0800, Ding Hui wrote:
> On 2021/6/14 10:12, Naoya Horiguchi wrote:
> > From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> > 
> > After recent soft-offline rework, error pages can be taken off from
> > buddy allocator, but the existing unpoison_memory() does not properly
> > undo the operation.  Moreover, due to the recent change on
> > __get_hwpoison_page(), get_page_unless_zero() is hardly called for
> > hwpoisoned pages.  So __get_hwpoison_page() mostly returns zero (meaning
> > to fail to grab page refcount) and unpoison just clears PG_hwpoison
> > without releasing a refcount.  That does not lead to a critical issue
> > like kernel panic, but unpoisoned pages never get back to buddy (leaked
> > permanently), which is not good.
> 
> As I mention in [1], I'm not sure about the exactly meaning of "broken" in
> unpoison_memory().
> 
> Maybe the misunderstanding is:
> 
> I think __get_hwpoison_page() mostly returns one for hwpoisoned page.
> In 06be6ff3d2ec ("mm,hwpoison: rework soft offline for free pages"),
> page_handle_poison() is introduced, it will add refcount for all
> soft-offlineed hwpoison page.
> In memory_failure() for hard-offline，page_ref_inc() called on free page
> too, and for used page, we do not call put_page() after get_hwpoison_page()
> != 0.
> So all hwpoisoned page refcount must be great than zero when
> unpoison_memory() if regardless of racy.

Hi, Ding,

Thanks for the comment.  I feel that I failed to define the exact issue in
unpoison.  Maybe I saw and misinterpreted some random error as unpoison's
issue during developing other hwpoison patches, so please don't take serious
my previous wrong word "broken", sorry about that.

Anyway I reconsider how to handle this 6/6, maybe it will be a clear
description of the problem, and will be simplified.

> 
> Recently I tested loop soft-offline random pages and unpoison them for days,
> it works fine to me. (with bac9c6fa1f92 patched)

Thank you for testing,

Thanks,
Naoya Horiguchi