From: Shakeel Butt <shakeelb@google.com>
To: Yang Shi <shy828301@gmail.com>, Yu Zhao <yuzhao@google.com>
Cc: Hugh Dickins <hughd@google.com>,
Matthew Wilcox <willy@infradead.org>,
Vladimir Davydov <vdavydov.dev@gmail.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Linux MM <linux-mm@kvack.org>
Subject: Re: Idle THPs
Date: Thu, 17 Jun 2021 08:57:52 -0700 [thread overview]
Message-ID: <CALvZod5FB1oTUHcMroehR7zdOp-uo6aHSEJh8SqZ-C_godWnzw@mail.gmail.com> (raw)
In-Reply-To: <CAHbLzkrznNBhGHZCN-Pf=1tUK+9Ad0TEXkC_fwDNcjceDt3vuw@mail.gmail.com>
+Yu Zhao
On Thu, Jun 10, 2021 at 3:54 PM Yang Shi <shy828301@gmail.com> wrote:
>
> On Thu, Jun 10, 2021 at 2:47 PM Hugh Dickins <hughd@google.com> wrote:
> >
> > On Thu, 10 Jun 2021, Matthew Wilcox wrote:
> >
> > > As part of the folio work, I'm looking at PageIdle and PageYoung and
> > > they're defined to operate on PF_ANY. So, for example, in
> > > pagecache_get_page(), we will call clear_page_idle() on the head page
> > > (actually, I changed this in a8cf7f272b5a -- before, it would call
> > > clear_page_idle() on the tail page).
> > >
> > > However, we never actually call set_page_idle() on tail pages. This is
> > > because we only call it here:
> > >
> > > page = page_idle_get_page(pfn);
> > > if (page) {
> > > page_idle_clear_pte_refs(page);
> > > set_page_idle(page);
> > > put_page(page);
> > > }
> > >
> > > where page_idle_get_page() does:
> > >
> > > struct page *page = pfn_to_online_page(pfn);
> > >
> > > if (!page || !PageLRU(page) ||
> > > !get_page_unless_zero(page))
> > > return NULL;
> > >
> > > get_page_unless_zero() will always fail for tail pages (as it uses
> > > page_ref_add_unless(), which does not redirect to the head page's
> > > refcount). So all tail pages read back as !idle in
> > > page_idle_bitmap_read(). Is this intended? Should they rather
> > > mirror the state of their head page?
From what I understand the idle bitmap is supposed to be used along
with /proc/kpageflags. So, the users should skip tail pages for
setting/getting the idle bits.
> >
> > Good point.
> >
> > I bet when I made that no-lru_lock cleanup in page_idle_get_page(),
> > I was expecting the PageLRU to fail on tail, so get_page_unless_zero()
> > irrelevant; but apparently PageLRU is PF_HEAD redirecting to head.
> > Either way, yes, it will return NULL on tail, which may not be right.
> >
> > But maybe the physical scan works out okay with all the action
> > happening on the head (Kirill got that to scan the tails in pvmw),
> > then skipping the tails in the local scan.
> >
> > I'm not a page_idle user and don't want to get into the mechanics of it.
> > Seems to be largely in maintenance mode these days, maybe nobody cares.
> >
> > Yang Shi was the last to make a real mod there, f0849ac0b8e0 ("mm: thp: fix
> > potential clearing to referenced flag in page_idle_clear_pte_refs_one()"):
> > likely he will know best.
>
> It was more than 3 years ago :-)
>
> Since the whole THP is considered referenced if any one of sub page is
> referenced, so IMHO the tail page should mirror the state of their
> head page.
>
> And AFAIK Google uses the idle flag to reclaim memory proactively, but
> it is an out-of-tree feature. Loop Shakeel in this thread.
We are not directly using upstream idle page infrastructure but
surgically using the parts we need. Yu can provide more details.
>
> >
> > Hugh
prev parent reply other threads:[~2021-06-17 15:58 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 3:43 Idle THPs Matthew Wilcox
2021-06-14 8:16 ` SeongJae Park
[not found] ` <59f61523-cb38-bf8c-51ba-1017ea7212d2@google.com>
[not found] ` <CAHbLzkrznNBhGHZCN-Pf=1tUK+9Ad0TEXkC_fwDNcjceDt3vuw@mail.gmail.com>
2021-06-17 15:57 ` Shakeel Butt [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=CALvZod5FB1oTUHcMroehR7zdOp-uo6aHSEJh8SqZ-C_godWnzw@mail.gmail.com \
--to=shakeelb@google.com \
--cc=hughd@google.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=shy828301@gmail.com \
--cc=vdavydov.dev@gmail.com \
--cc=willy@infradead.org \
--cc=yuzhao@google.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).