PageLRU and the other flags

PageLRU and the other flags

[Matthew Wilcox]

I'm not quite familiar with this side of the MM yet, but it seems to
me like we're encoding four page states in three bits:

 - !PageLRU (not an LRU page)
 - PageLRU (on the inactive list)
 - PageLRU + PageActive (on the active list)
 - PageLRU + PageUnevictable (on the unevictable list)

Can we neaten this up into two bits?

 - 00 (not an LRU page)
 - 01 (active list)
 - 10 (inactive list)
 - 11 (unevictable)

People who free up page flags are always popular, right?

I don't think the missing combos are used:

!LRU + Active
!LRU + Active + Unevictable
!LRU + Unevictable
LRU + Active + Unevictable

It seems fairly straightforward to add these macros:

bool PageLRU(struct page *page)
{
	page->flags & PAGE_FLAGS_LRU_MASK;
}

bool PageActive(struct page *page)
{
	page->flags & PAGE_FLAGS_LRU_MASK == PAGE_FLAGS_ACTIVE;
}

bool PageInactive(struct page *page)
{
	page->flags & PAGE_FLAGS_LRU_MASK == PAGE_FLAGS_INACTIVE;
}

bool PageUnevictable(struct page *page)
{
	page->flags & PAGE_FLAGS_LRU_MASK == PAGE_FLAGS_UNEVICTABLE;
}

SetPageActive seems a little more tricky to do atomically.  A cmpxchg()
loop, perhaps?  If we changed the API to be PageUnevictableToActive(page)
then it's an atomic subtraction / addition (although it's now twelve
macro instead of six Set/Clear for three bits).  Then I'm not sure how
to do TestClearPageActive().  Maybe TryPageActiveToInactive()?  That'd be
another twelve macros if all those state transitions are possible.

Re: PageLRU and the other flags

[David Hildenbrand]

On 25.09.20 17:00, Matthew Wilcox wrote:
> I'm not quite familiar with this side of the MM yet, but it seems to
> me like we're encoding four page states in three bits:
> 
>  - !PageLRU (not an LRU page)
>  - PageLRU (on the inactive list)
>  - PageLRU + PageActive (on the active list)
>  - PageLRU + PageUnevictable (on the unevictable list)
> 
> Can we neaten this up into two bits?
> 
>  - 00 (not an LRU page)
>  - 01 (active list)
>  - 10 (inactive list)
>  - 11 (unevictable)
> 
> People who free up page flags are always popular, right?

They are the best :)

Looking at __pagevec_lru_add_fn()

SetPageLRU(page);
smp_mb__after_atomic();
...

if (page_evictable(page)) {
	...
} else {
	lru = LRU_UNEVICTABLE;
	ClearPageActive(page);
	SetPageUnevictable(page);
	...
}

Looks like PageActive(page) could have been relevant even without
SetPageLRU(page);

So the combo

!PageLRU() + PageActive (previously was on the active list maybe?)

might be valid? Or dead code? Or valid races (someone else spotting
PageLRU() and setting it PageActive(page))?

But no expert, just blindly poking around ...

-- 
Thanks,

David / dhildenb

Re: PageLRU and the other flags

[Yang Shi]

On Fri, Sep 25, 2020 at 8:21 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 25.09.20 17:00, Matthew Wilcox wrote:
> > I'm not quite familiar with this side of the MM yet, but it seems to
> > me like we're encoding four page states in three bits:
> >
> >  - !PageLRU (not an LRU page)
> >  - PageLRU (on the inactive list)
> >  - PageLRU + PageActive (on the active list)
> >  - PageLRU + PageUnevictable (on the unevictable list)
> >
> > Can we neaten this up into two bits?
> >
> >  - 00 (not an LRU page)
> >  - 01 (active list)
> >  - 10 (inactive list)
> >  - 11 (unevictable)
> >
> > People who free up page flags are always popular, right?
>
> They are the best :)
>
> Looking at __pagevec_lru_add_fn()
>
> SetPageLRU(page);
> smp_mb__after_atomic();
> ...
>
> if (page_evictable(page)) {
>         ...
> } else {
>         lru = LRU_UNEVICTABLE;
>         ClearPageActive(page);
>         SetPageUnevictable(page);
>         ...
> }
>
> Looks like PageActive(page) could have been relevant even without
> SetPageLRU(page);
>
> So the combo
>
> !PageLRU() + PageActive (previously was on the active list maybe?)
>
> might be valid? Or dead code? Or valid races (someone else spotting
> PageLRU() and setting it PageActive(page))?

Yes, it is valid. The page might be marked active when it is on
pagevec, then will be moved to active LRU in the next drain. Please
refer to the comment in mark_page_accessed().

> But no expert, just blindly poking around ...
>
> --
> Thanks,
>
> David / dhildenb
>
>