diff mbox series

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

Message ID 20190625075227.15193-4-osalvador@suse.de (mailing list archive)
State New, archived
Headers show
Series Allocate memmap from hotadded memory | expand

Commit Message

Oscar Salvador June 25, 2019, 7:52 a.m. UTC
Introduce a set of functions for Vmemmap pages.
Set of functions:

- {Set,Clear,Check} Vmemmap flag
- Given a vmemmap page, get its vmemmap-head
- Get #nr of vmemmap pages taking into account the current position
  of the page

These functions will be used for the code handling Vmemmap pages.

Signed-off-by: Oscar Salvador <osalvador@suse.de>
---
 include/linux/page-flags.h | 34 ++++++++++++++++++++++++++++++++++
 mm/util.c                  |  2 ++
 2 files changed, 36 insertions(+)

Comments

David Hildenbrand June 25, 2019, 10:28 a.m. UTC | #1
On 25.06.19 09:52, Oscar Salvador wrote:
> Introduce a set of functions for Vmemmap pages.
> Set of functions:
> 
> - {Set,Clear,Check} Vmemmap flag
> - Given a vmemmap page, get its vmemmap-head
> - Get #nr of vmemmap pages taking into account the current position
>   of the page
> 
> These functions will be used for the code handling Vmemmap pages.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
>  include/linux/page-flags.h | 34 ++++++++++++++++++++++++++++++++++
>  mm/util.c                  |  2 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index b848517da64c..a8b9b57162b3 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -466,6 +466,40 @@ static __always_inline int __PageMovable(struct page *page)
>  				PAGE_MAPPING_MOVABLE;
>  }
>  
> +#define VMEMMAP_PAGE		(~PAGE_MAPPING_FLAGS)
> +static __always_inline int PageVmemmap(struct page *page)
> +{
> +	return PageReserved(page) && (unsigned long)page->mapping == VMEMMAP_PAGE;
> +}
> +
> +static __always_inline int __PageVmemmap(struct page *page)
> +{
> +	return (unsigned long)page->mapping == VMEMMAP_PAGE;
> +}
> +
> +static __always_inline void __ClearPageVmemmap(struct page *page)
> +{

Should we VM_BUG_ON in case !PG_reserved || pg->mapping != VMEMMAP_PAGE ?

> +	__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 ?

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

> +}
> +
> +static __always_inline unsigned long get_nr_vmemmap_pages(struct page *page)
> +{
> +	struct page *head = vmemmap_get_head(page);
> +	return head->private - (page - head);
> +}
> +
>  #ifdef CONFIG_KSM
>  /*
>   * A KSM page is one of those write-protected "shared pages" or "merged pages"
> diff --git a/mm/util.c b/mm/util.c
> index 021648a8a3a3..5e20563cdef6 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -607,6 +607,8 @@ struct address_space *page_mapping(struct page *page)
>  	mapping = page->mapping;
>  	if ((unsigned long)mapping & PAGE_MAPPING_ANON)
>  		return NULL;
> +	if ((unsigned long)mapping == VMEMMAP_PAGE)
> +		return NULL;
>  
>  	return (void *)((unsigned long)mapping & ~PAGE_MAPPING_FLAGS);
>  }
> 
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.
Oscar Salvador June 26, 2019, 9:48 a.m. UTC | #2
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.
diff mbox series

Patch

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index b848517da64c..a8b9b57162b3 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -466,6 +466,40 @@  static __always_inline int __PageMovable(struct page *page)
 				PAGE_MAPPING_MOVABLE;
 }
 
+#define VMEMMAP_PAGE		(~PAGE_MAPPING_FLAGS)
+static __always_inline int PageVmemmap(struct page *page)
+{
+	return PageReserved(page) && (unsigned long)page->mapping == VMEMMAP_PAGE;
+}
+
+static __always_inline int __PageVmemmap(struct page *page)
+{
+	return (unsigned long)page->mapping == VMEMMAP_PAGE;
+}
+
+static __always_inline void __ClearPageVmemmap(struct page *page)
+{
+	__ClearPageReserved(page);
+	page->mapping = NULL;
+}
+
+static __always_inline void __SetPageVmemmap(struct page *page)
+{
+	__SetPageReserved(page);
+	page->mapping = (void *)VMEMMAP_PAGE;
+}
+
+static __always_inline struct page *vmemmap_get_head(struct page *page)
+{
+	return (struct page *)page->freelist;
+}
+
+static __always_inline unsigned long get_nr_vmemmap_pages(struct page *page)
+{
+	struct page *head = vmemmap_get_head(page);
+	return head->private - (page - head);
+}
+
 #ifdef CONFIG_KSM
 /*
  * A KSM page is one of those write-protected "shared pages" or "merged pages"
diff --git a/mm/util.c b/mm/util.c
index 021648a8a3a3..5e20563cdef6 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -607,6 +607,8 @@  struct address_space *page_mapping(struct page *page)
 	mapping = page->mapping;
 	if ((unsigned long)mapping & PAGE_MAPPING_ANON)
 		return NULL;
+	if ((unsigned long)mapping == VMEMMAP_PAGE)
+		return NULL;
 
 	return (void *)((unsigned long)mapping & ~PAGE_MAPPING_FLAGS);
 }