All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jane Chu <jane.chu@oracle.com>
To: Matthew Wilcox <willy@infradead.org>,
	Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, linmiaohe@huawei.com,
	nao.horiguchi@gmail.com, osalvador@suse.de
Subject: Re: [PATCH] mm/memory-failure: remove shake_page()
Date: Fri, 26 Apr 2024 13:33:27 -0700	[thread overview]
Message-ID: <d19e1033-285f-474b-af1f-ae4c32e32e21@oracle.com> (raw)
In-Reply-To: <47614f5d-0942-4b2c-a51a-451f6bc9c802@oracle.com>

My apology for the gobbled message earlier.

On 4/26/2024 12:52 PM, Jane Chu wrote:
> On 4/26/2024 12:05 PM, Matthew Wilcox wrote:
> [..]
>> That would be unsafe, the safe way would be if we moved page_folio() 
>> after
>>> the call to __get_hw_poison() in get_any_page() and there would 
>>> still be one
>>> remaining user of shake_page() that we can't convert. A safe version 
>>> of this
>>> patch would result in a removal of one use of PageHuge() and two 
>>> uses of
>>> put_page(), would that be worth submitting?
>>>
>>> get_any_page()
>>>     if(__get_hwpoison_page())
>>>         folio = page_folio() /* folio_try_get() returned 1, safe */
>> I think we should convert __get_hwpoison_page() to return either the 
>> folio
>> or an ERR_PTR or NULL.  Also, I think we should delete the "cannot catch
>> tail" part and just loop in __get_hwpoison_page() until we do catch it.
>> See try_get_folio() in mm/gup.c for inspiration (although you can't use
>> it exactly because that code knows that the page is mapped into a page
>> table, so has a refcount).
>>
>> But that's just an immediate assessment; you might find a reason that
>> doesn't work.
>
Besides, in a possible hugetlb demote scenario, it seems to me that we 
should retry
get_hwpoison_hugetlb_folio() instead of falling thru to folio_try_get().

static int __get_hwpoison_page(struct page *page, unsigned long flags)
{
         struct folio *folio = page_folio(page);
         int ret = 0;
         bool hugetlb = false;

         ret = get_hwpoison_hugetlb_folio(folio, &hugetlb, false);
         if (hugetlb) {
                 /* Make sure hugetlb demotion did not happen from under 
us. */
                 if (folio == page_folio(page))
                         return ret;
                 if (ret > 0) {      <===== still hugetlb, don't fall 
thru, retry
                         folio_put(folio);
                         folio = page_folio(page);
                 }

         }

thanks!
-jane

  reply	other threads:[~2024-04-26 20:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 17:15 [PATCH] mm/memory-failure: remove shake_page() Sidhartha Kumar
2024-04-26 17:34 ` Matthew Wilcox
2024-04-26 17:57   ` Sidhartha Kumar
2024-04-26 18:27     ` Matthew Wilcox
2024-04-26 18:53       ` Sidhartha Kumar
2024-04-26 19:05         ` Matthew Wilcox
2024-04-26 19:52           ` Jane Chu
2024-04-26 20:33             ` Jane Chu [this message]
2024-04-28  2:24               ` 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=d19e1033-285f-474b-af1f-ae4c32e32e21@oracle.com \
    --to=jane.chu@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=linmiaohe@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nao.horiguchi@gmail.com \
    --cc=osalvador@suse.de \
    --cc=sidhartha.kumar@oracle.com \
    --cc=willy@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.