linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Naoya Horiguchi <naoya.horiguchi@linux.dev>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Muchun Song <songmuchun@bytedance.com>,
	Miaohe Lin <linmiaohe@huawei.com>,
	Matthew Wilcox <willy@infradead.org>,
	Yang Shi <shy828301@gmail.com>,
	Naoya Horiguchi <naoya.horiguchi@nec.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm/huge_memory: use pfn_to_online_page() in split_huge_pages_all()
Date: Thu, 8 Sep 2022 18:48:38 +0200	[thread overview]
Message-ID: <Yxoc5mrIjrh+I5w5@dhcp22.suse.cz> (raw)
In-Reply-To: <20220908122303.7pofcdcmbuq4ou6u@box.shutemov.name>

On Thu 08-09-22 15:23:03, Kirill A. Shutemov wrote:
[...]
> But it makes me think if there's other similar cases. "page is offline" is
> rather obscure case that rarely covered by routine testing. Otherwise the
> bug would not survive for 6 years.
> 
> After quick look, kvm_pfn_to_refcounted_page() looks suspicious.

this one is hard to judge for me. Is this ever used for something that
could be offlined?

> kdb_getphys() too.

this one looks it needs a fix
 
> Maybe we should make pfn_valid() false for offline pages and introduce
> other check that allows offline pages which can be used in codepaths that
> deal with offline pages explicitly.

Back when pfn_to_online page was introduced altering pfn_valid was
considered but the semantic is not the same. offline pages could be
still pfn_valid. The discussion should be in archives. Sorry do not have
the link handy. We have also considered changing pfn_to_page but that
would add an overhead for unlikely case to everybody. So the conclusion
was that only pfn walkers should care. Most other users just translate
pfn to page when the page cannot be offlined (e.g. because it is
referenced).

I do realize this is fragile but we couldn't come up with something more
clever without introducing overhead all over the place.
-- 
Michal Hocko
SUSE Labs


      reply	other threads:[~2022-09-08 16:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-08  4:11 [PATCH v2] mm/huge_memory: use pfn_to_online_page() in split_huge_pages_all() Naoya Horiguchi
2022-09-08 12:23 ` Kirill A. Shutemov
2022-09-08 16:48   ` Michal Hocko [this message]

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=Yxoc5mrIjrh+I5w5@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=kirill@shutemov.name \
    --cc=linmiaohe@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=naoya.horiguchi@linux.dev \
    --cc=naoya.horiguchi@nec.com \
    --cc=shy828301@gmail.com \
    --cc=songmuchun@bytedance.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 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).