[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f0ac7747-0e18-5039-d341-5dfda8d5780e@redhat.com>
Date: Thu, 25 Jul 2019 18:48:35 +0200
From: David Hildenbrand <david@...hat.com>
To: Alexander Duyck <alexander.duyck@...il.com>
Cc: Nitesh Narayan Lal <nitesh@...hat.com>,
kvm list <kvm@...r.kernel.org>,
"Michael S. Tsirkin" <mst@...hat.com>,
Dave Hansen <dave.hansen@...el.com>,
LKML <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Yang Zhang <yang.zhang.wz@...il.com>, pagupta@...hat.com,
Rik van Riel <riel@...riel.com>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
lcapitulino@...hat.com, wei.w.wang@...el.com,
Andrea Arcangeli <aarcange@...hat.com>,
Paolo Bonzini <pbonzini@...hat.com>, dan.j.williams@...el.com,
Matthew Wilcox <willy@...radead.org>,
Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Subject: Re: [PATCH v2 4/5] mm: Introduce Hinted pages
On 25.07.19 17:59, Alexander Duyck wrote:
> On Thu, Jul 25, 2019 at 1:53 AM David Hildenbrand <david@...hat.com> wrote:
>>
>> On 24.07.19 19:03, Alexander Duyck wrote:
>>> From: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
>
> <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
Powered by blists - more mailing lists