Re: [PATCH v2 4/5] mm: Introduce Hinted pages

From: Alexander Duyck
Date: Thu Jul 25 2019 - 14:09:04 EST


On Thu, 2019-07-25 at 18:48 +0200, David Hildenbrand wrote:
> On 25.07.19 17:59, Alexander Duyck wrote:
> > On Thu, Jul 25, 2019 at 1:53 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
> > > On 24.07.19 19:03, Alexander Duyck wrote:
> > > > From: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
> >

<snip>

> > > Can't we reuse one of the traditional page flags for that, not used
> > > along with buddy pages? E.g., PG_dirty: Pages that were not hinted yet
> > > are dirty.
> >
> > Reusing something like the dirty bit would just be confusing in my
> > opinion. In addition it looks like Xen has also re-purposed PG_dirty
> > already for another purpose.
>
> You brought up waste page management. A dirty bit for unprocessed pages
> fits perfectly in this context. Regarding XEN, as long as it's not used
> along with buddy pages, no issue.

I would rather not have to dirty all pages that aren't hinted. That starts
to get too invasive. Ideally we only modify pages if we are hinting on
them. That is why I said I didn't like the use of a dirty bit. What we
want is more of a "guaranteed clean" bit.

> FWIW, I don't even thing PG_offline matches to what you are using it
> here for. The pages are not logically offline. They were simply buddy
> pages that were hinted. (I'd even prefer a separate page type for that
> instead - if we cannot simply reuse one of the other flags)
>
> "Offline pages" that are not actually offline in the context of the
> buddy is way more confusing.

Right now offline and hinted are essentially the same thing since the
effect is identical.

There may be cases in the future where that is not the case, but with the
current patch set they both result in the pages being evicted from the
guest.

> > If anything I could probably look at seeing if the PG_private flags
> > are available when a page is in the buddy allocator which I suspect
> > they probably are since the only users I currently see appear to be
> > SLOB and compound pages. Either that or maybe something like PG_head
> > might make sense since once we start allocating them we are popping
> > the head off of the boundary list.
>
> Would also be fine with me.

Actually I may have found an even better bit if we are going with the
"reporting" name. I could probably use "PG_uptodate" since it looks like
most of its uses are related to filesystems. I will wait till I hear from
Matthew on what bits would be available for use before I update things.