linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "HORIGUCHI NAOYA(堀口 直也)" <naoya.horiguchi@nec.com>
To: Miaohe Lin <linmiaohe@huawei.com>
Cc: Naoya Horiguchi <naoya.horiguchi@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Liu Shixin <liushixin2@huawei.com>,
	Yang Shi <shy828301@gmail.com>,
	Oscar Salvador <osalvador@suse.de>,
	Muchun Song <songmuchun@bytedance.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH v1 3/5] mm, hwpoison: make __page_handle_poison returns int
Date: Wed, 8 Jun 2022 01:40:16 +0000	[thread overview]
Message-ID: <20220608014015.GB1404644@hori.linux.bs1.fc.nec.co.jp> (raw)
In-Reply-To: <9d423acb-b671-1cf0-0432-76eb648de95b@huawei.com>

On Tue, Jun 07, 2022 at 08:54:24PM +0800, Miaohe Lin wrote:
> On 2022/6/2 13:06, Naoya Horiguchi wrote:
> > From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> > 
> > __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@nec.com>
> > ---
> >  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.

Ah, you're right. this patch is stated as "non functional change" but that
is not true due to this.  So I'll move this line deletion to 4/5 in the next
version.

> 
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> Thanks!

Thank you :)

- Naoya Horiguchi

  reply	other threads:[~2022-06-08  1:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-02  5:06 [PATCH v1 0/5] mm, hwpoison: enable 1GB hugepage support Naoya Horiguchi
2022-06-02  5:06 ` [PATCH v1 1/5] mm, hwpoison, hugetlb: introduce SUBPAGE_INDEX_HWPOISON to save raw error page Naoya Horiguchi
2022-06-07 12:45   ` Miaohe Lin
2022-06-07 13:04   ` David Hildenbrand
2022-06-08  1:31     ` HORIGUCHI NAOYA(堀口 直也)
2022-06-08  9:19       ` David Hildenbrand
2022-06-02  5:06 ` [PATCH v1 2/5] mm,hwpoison: set PG_hwpoison for busy hugetlb pages Naoya Horiguchi
2022-06-07 12:50   ` Miaohe Lin
2022-06-02  5:06 ` [PATCH v1 3/5] mm, hwpoison: make __page_handle_poison returns int Naoya Horiguchi
2022-06-07 12:54   ` Miaohe Lin
2022-06-08  1:40     ` HORIGUCHI NAOYA(堀口 直也) [this message]
2022-06-02  5:06 ` [PATCH v1 4/5] mm, hwpoison: skip raw hwpoison page in freeing 1GB hugepage Naoya Horiguchi
2022-06-07 13:56   ` Miaohe Lin
2022-06-02  5:06 ` [PATCH v1 5/5] mm, hwpoison: enable memory error handling on " Naoya Horiguchi
2022-06-07 14:11   ` Miaohe Lin
2022-06-08  6:16     ` HORIGUCHI NAOYA(堀口 直也)
2022-06-08 12:57       ` Miaohe Lin
2022-06-09  8:45         ` HORIGUCHI NAOYA(堀口 直也)
2022-06-09 11:28           ` Miaohe Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220608014015.GB1404644@hori.linux.bs1.fc.nec.co.jp \
    --to=naoya.horiguchi@nec.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linmiaohe@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liushixin2@huawei.com \
    --cc=mike.kravetz@oracle.com \
    --cc=naoya.horiguchi@linux.dev \
    --cc=osalvador@suse.de \
    --cc=shy828301@gmail.com \
    --cc=songmuchun@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).