Re: Idle THPs

+Yu Zhao

On Thu, Jun 10, 2021 at 3:54 PM Yang Shi [off-list ref] wrote:

On Thu, Jun 10, 2021 at 2:47 PM Hugh Dickins [off-list ref] wrote:

quoted

On Thu, 10 Jun 2021, Matthew Wilcox wrote:

quoted

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.

quoted

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.

quoted

Hugh