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

[David Hildenbrand]

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>
> 
>>>  /*
>>> + * PageHinted() is an alias for Offline, however it is not meant to be an
>>> + * exclusive value. It should be combined with PageBuddy() when seen as it
>>> + * is meant to indicate that the page has been scrubbed while waiting in
>>> + * the buddy system.
>>> + */
>>> +PAGE_TYPE_OPS(Hinted, offline)
>>
>>
>> CCing Matthew
>>
>> I am still not sure if I like the idea of having two page types at a time.
>>
>> 1. Once we run out of page type bits (which can happen easily looking at
>> it getting more and more user - e.g., maybe for vmmap pages soon), we
>> might want to convert again back to a value-based, not bit-based type
>> detection. This will certainly make this switch harder.
> 
> Shouldn't we wait to cross that bridge until we get there? It wouldn't
> take much to look at either defining the buddy as 2 types for such a
> case, or if needed we could then look at the option of moving over to
> another bit.

I'd rather clarify this now. I am not yet convinced that having multiple
page types at a is a good idea.

> 
>> 2. It will complicate the kexec/kdump handling. I assume it can be fixed
>> some way - e.g., making the elf interface aware of the exact notion of
>> page type bits compared to mapcount values we have right now (e.g.,
>> PAGE_BUDDY_MAPCOUNT_VALUE). Not addressed in this series yet.
> 
> It does, but not by much. We were already exposing both the buddy and
> offline values. The cahnge could probably be in the executable that
> are accessing the interface to allow the combination of buddy and
> offline.

We are exposing mapcount values, not bit values. So you would

> That is one of the advantages of using the "offline" value to
> also mean hinted since then "hinted" is just a combination of the two
> known values.

We are exposing mapcount values right now, not individual bits. Either
expose the bits manually (and thereby the whole page type scheme) or a
new mapcount value PAGE_BUDDY_OFFLINE_MAPCOUNT_VALUE.

> 
>> 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.

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.

> 
> 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.

-- 

Thanks,

David / dhildenb