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
