Re: [PATCH v2 3/5] mm,memory_hotplug: Introduce Vmemmap page helpers

[Oscar Salvador <osalvador@suse.de>]

On Tue, Jun 25, 2019 at 12:28:56PM +0200, David Hildenbrand wrote:
> > +static __always_inline void __ClearPageVmemmap(struct page *page)
> > +{
> 
> Should we VM_BUG_ON in case !PG_reserved || pg->mapping != VMEMMAP_PAGE ?

We can do that, just for extra protection.

> 
> > +	__ClearPageReserved(page);
> > +	page->mapping = NULL;
> > +}
> > +
> > +static __always_inline void __SetPageVmemmap(struct page *page)
> > +{
> 
> Should we VM_BUG_ON in case PG_reserved || pg->mapping != NULL ?

ditto.

> 
> > +	__SetPageReserved(page);
> > +	page->mapping = (void *)VMEMMAP_PAGE;
> > +}
> > +
> > +static __always_inline struct page *vmemmap_get_head(struct page *page)
> > +{
> > +	return (struct page *)page->freelist;
> 
> freelist is a "slab, slob and slub" concept (reading
> include/linux/mm_types.h). page->mapping is a "Page cache and anonymous
> pages" concept. Hmmm...

Yeah.
In an early stage, I thought about constructing vmemmap pages the same way
we construct compound pages, so we can leverage the APIs that we already have.

For some reason I did not go further with that, but I will investigate in that
direction.

> I wonder if using a page type would be appropriate here instead. Then,
> define a new sub-structure within "struct page" that describes what you
> actually want (instead of reusing ->private and ->mapping). Just an
> idea, we have to find out if that is possible.
> 
> vmemmap_get_head() smells like __GFP_COMP, but of course, these vmemmap
> pages never saw the buddy. But sounds like you want a similar concept.

Thanks for the feedback.
I will take a look at it.

-- 
Oscar Salvador
SUSE L3