From: Oscar Salvador <osalvador@suse.de>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Michal Hocko <mhocko@kernel.org>,
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
Muchun Song <songmuchun@bytedance.com>,
David Hildenbrand <david@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [RFC PATCH 2/3] hugetlb: convert page_huge_active() to HPageMigratable flag
Date: Fri, 15 Jan 2021 10:17:56 +0100 [thread overview]
Message-ID: <20210115091755.GB4092@linux> (raw)
In-Reply-To: <20210111210152.118394-3-mike.kravetz@oracle.com>
On Mon, Jan 11, 2021 at 01:01:51PM -0800, Mike Kravetz wrote:
> Use the new hugetlb page specific flag to replace the page_huge_active
> interfaces. By it's name, page_huge_active implied that a huge page
> was on the active list. However, that is not really what code checking
> the flag wanted to know. It really wanted to determine if the huge
> page could be migrated. This happens when the page is actually added
> the page cache and/or task page table. This is the reasoning behind the
> name change.
>
> The VM_BUG_ON_PAGE() calls in the interfaces were not really necessary
> as in all case but one we KNOW the page is a hugetlb page. Therefore,
> they are removed. In one call to HPageMigratable() is it possible for
> the page to not be a hugetlb page due to a race. However, the code
> making the call (scan_movable_pages) is inherently racy, and page state
> will be validated later in the migration process.
>
> Note: Since HPageMigratable is used outside hugetlb.c, it can not be
> static. Therefore, a new set of hugetlb page flag macros is added for
> non-static flag functions.
Two things about this one:
I am not sure about the name of this one.
It is true that page_huge_active() was only called by memory-hotplug and all
it wanted to know was whether the page was in-use and so if it made sense
to migrate it, so I see some value in the new PageMigratable flag.
However, not all in-use hugetlb can be migrated, e.g: we might have constraints
when it comes to migrate certain sizes of hugetlb, right?
So setting HPageMigratable to all active hugetlb pages might be a bit misleading?
HPageActive maybe? (Sorry, don't have a replacement)
The other thing is that you are right that scan_movable_pages is racy, but
page_huge_active() was checking if the page had the Head flag set before
retrieving page[1].
Before the page_huge_active() in scan_movable_pages() we have the
if (!PageHuge(page)) check, but could it be that between that check and
the page_huge_active(), the page gets dissolved, and so we are checking
a wrong page[1]? Am I making sense?
--
Oscar Salvador
SUSE L3
next prev parent reply other threads:[~2021-01-15 9:18 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 21:01 [RFC PATCH 0/3] create hugetlb flags to consolidate state Mike Kravetz
2021-01-11 21:01 ` [RFC PATCH 1/3] hugetlb: use page.private for hugetlb specific page flags Mike Kravetz
2021-01-12 3:24 ` [External] " Muchun Song
2021-01-13 13:54 ` Oscar Salvador
2021-01-13 17:49 ` Mike Kravetz
2021-01-13 14:45 ` Matthew Wilcox
2021-01-13 17:51 ` Mike Kravetz
2021-01-11 21:01 ` [RFC PATCH 2/3] hugetlb: convert page_huge_active() to HPageMigratable flag Mike Kravetz
2021-01-12 3:45 ` [External] " Muchun Song
2021-01-15 9:17 ` Oscar Salvador [this message]
2021-01-15 17:43 ` Mike Kravetz
2021-01-15 20:05 ` Mike Kravetz
2021-01-15 20:29 ` Oscar Salvador
2021-01-15 21:25 ` Mike Kravetz
2021-01-15 20:38 ` Oscar Salvador
2021-01-11 21:01 ` [RFC PATCH 3/3] hugetlb: convert PageHugeTemporary() to HPageTempSurplus Mike Kravetz
2021-01-15 10:16 ` Oscar Salvador
2021-01-15 17:47 ` Mike Kravetz
2021-01-12 10:41 ` [RFC PATCH 0/3] create hugetlb flags to consolidate state David Hildenbrand
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=20210115091755.GB4092@linux \
--to=osalvador@suse.de \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=n-horiguchi@ah.jp.nec.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).