All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: Split page_type out from _map_count
@ 2018-02-07 21:30 Matthew Wilcox
  2018-02-09 10:51 ` Kirill A. Shutemov
  2018-02-10  5:00 ` [PATCH] " kbuild test robot
  0 siblings, 2 replies; 8+ messages in thread
From: Matthew Wilcox @ 2018-02-07 21:30 UTC (permalink / raw)
  To: linux-mm; +Cc: Matthew Wilcox

From: Matthew Wilcox <mawilcox@microsoft.com>

We're already using a union of many fields here, so stop abusing the
_map_count and make page_type its own field.  That implies renaming some
of the machinery that creates PageBuddy, PageBalloon and PageKmemcg;
bring back the PG_buddy, PG_balloon and PG_kmemcg names.  Also, the
special values don't need to be (and probably shouldn't be) powers of two,
so renumber them to just start at -128.

Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
 include/linux/mm_types.h   | 13 ++++++++-----
 include/linux/page-flags.h | 36 +++++++++++++++++-------------------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index fd1af6b9591d..1c5dea402501 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -94,6 +94,14 @@ struct page {
 	};
 
 	union {
+		/*
+		 * If the page is neither PageSlab nor PageAnon, the value
+		 * stored here may help distinguish it from page cache pages.
+		 * See page-flags.h for a list of page types which are
+		 * currently stored here.
+		 */
+		unsigned int page_type;
+
 		_slub_counter_t counters;
 		unsigned int active;		/* SLAB */
 		struct {			/* SLUB */
@@ -107,11 +115,6 @@ struct page {
 			/*
 			 * Count of ptes mapped in mms, to show when
 			 * page is mapped & limit reverse map searches.
-			 *
-			 * Extra information about page type may be
-			 * stored here for pages that are never mapped,
-			 * in which case the value MUST BE <= -2.
-			 * See page-flags.h for more details.
 			 */
 			atomic_t _mapcount;
 
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 50c2b8786831..ba6a7e883425 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -630,49 +630,47 @@ PAGEFLAG_FALSE(DoubleMap)
 #endif
 
 /*
- * For pages that are never mapped to userspace, page->mapcount may be
- * used for storing extra information about page type. Any value used
- * for this purpose must be <= -2, but it's better start not too close
- * to -2 so that an underflow of the page_mapcount() won't be mistaken
- * for a special page.
+ * For pages that are never mapped to userspace, page_type may be
+ * used.  Values used for this purpose must be <= -2, but we leave a gap
+ * so that an underflow of page_mapcount() won't be mistaken for a
+ * special page.
  */
-#define PAGE_MAPCOUNT_OPS(uname, lname)					\
+#define PAGE_TYPE_OPS(uname, lname)					\
 static __always_inline int Page##uname(struct page *page)		\
 {									\
-	return atomic_read(&page->_mapcount) ==				\
-				PAGE_##lname##_MAPCOUNT_VALUE;		\
+	return page->page_type == PG_##lname;				\
 }									\
 static __always_inline void __SetPage##uname(struct page *page)		\
 {									\
-	VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);	\
-	atomic_set(&page->_mapcount, PAGE_##lname##_MAPCOUNT_VALUE);	\
+	VM_BUG_ON_PAGE(page->page_type != -1, page);			\
+	page->page_type = PG_##lname;					\
 }									\
 static __always_inline void __ClearPage##uname(struct page *page)	\
 {									\
 	VM_BUG_ON_PAGE(!Page##uname(page), page);			\
-	atomic_set(&page->_mapcount, -1);				\
+	page->page_type = -1;						\
 }
 
 /*
- * PageBuddy() indicate that the page is free and in the buddy system
+ * PageBuddy() indicates that the page is free and in the buddy system
  * (see mm/page_alloc.c).
  */
-#define PAGE_BUDDY_MAPCOUNT_VALUE		(-128)
-PAGE_MAPCOUNT_OPS(Buddy, BUDDY)
+#define PG_buddy		(-128)
+PAGE_TYPE_OPS(Buddy, buddy)
 
 /*
- * PageBalloon() is set on pages that are on the balloon page list
+ * PageBalloon() is true for pages that are on the balloon page list
  * (see mm/balloon_compaction.c).
  */
-#define PAGE_BALLOON_MAPCOUNT_VALUE		(-256)
-PAGE_MAPCOUNT_OPS(Balloon, BALLOON)
+#define PG_balloon		(-129)
+PAGE_TYPE_OPS(Balloon, balloon)
 
 /*
  * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
  * pages allocated with __GFP_ACCOUNT. It gets cleared on page free.
  */
-#define PAGE_KMEMCG_MAPCOUNT_VALUE		(-512)
-PAGE_MAPCOUNT_OPS(Kmemcg, KMEMCG)
+#define PG_kmemcg		(-130)
+PAGE_TYPE_OPS(Kmemcg, kmemcg)
 
 extern bool is_free_buddy_page(struct page *page);
 
-- 
2.15.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: Split page_type out from _map_count
  2018-02-07 21:30 [PATCH] mm: Split page_type out from _map_count Matthew Wilcox
@ 2018-02-09 10:51 ` Kirill A. Shutemov
  2018-02-09 13:49   ` Matthew Wilcox
  2018-02-10  5:00 ` [PATCH] " kbuild test robot
  1 sibling, 1 reply; 8+ messages in thread
From: Kirill A. Shutemov @ 2018-02-09 10:51 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: linux-mm, Matthew Wilcox

On Wed, Feb 07, 2018 at 01:30:47PM -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox <mawilcox@microsoft.com>
> 
> We're already using a union of many fields here, so stop abusing the
> _map_count and make page_type its own field.  That implies renaming some
> of the machinery that creates PageBuddy, PageBalloon and PageKmemcg;
> bring back the PG_buddy, PG_balloon and PG_kmemcg names.

Sounds reasonable to me.

> Also, the special values don't need to be (and probably shouldn't be) powers
> of two, so renumber them to just start at -128.

Are you sure about this? Is it guarantee that we would not need in the
future PG_buddy|PG_kmemcg for instance?

I guess we may want to make it a bitfield. In negative space it's kinda
interesting. :)

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: Split page_type out from _map_count
  2018-02-09 10:51 ` Kirill A. Shutemov
@ 2018-02-09 13:49   ` Matthew Wilcox
  2018-02-09 15:28     ` [PATCH v2] " Matthew Wilcox
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Wilcox @ 2018-02-09 13:49 UTC (permalink / raw)
  To: Kirill A. Shutemov; +Cc: linux-mm, Matthew Wilcox

On Fri, Feb 09, 2018 at 01:51:32PM +0300, Kirill A. Shutemov wrote:
> On Wed, Feb 07, 2018 at 01:30:47PM -0800, Matthew Wilcox wrote:
> > From: Matthew Wilcox <mawilcox@microsoft.com>
> > 
> > We're already using a union of many fields here, so stop abusing the
> > _map_count and make page_type its own field.  That implies renaming some
> > of the machinery that creates PageBuddy, PageBalloon and PageKmemcg;
> > bring back the PG_buddy, PG_balloon and PG_kmemcg names.
> 
> Sounds reasonable to me.
> 
> > Also, the special values don't need to be (and probably shouldn't be) powers
> > of two, so renumber them to just start at -128.
> 
> Are you sure about this? Is it guarantee that we would not need in the
> future PG_buddy|PG_kmemcg for instance?
> 
> I guess we may want to make it a bitfield. In negative space it's kinda
> interesting. :)

Far too interesting ;-)  We still wouldn't want it to be powers of two ...
We'd want to do something like:

#define PAGE_TYPE_BASE	0xfffff000
#define PG_buddy	0x00000080
#define PG_balloon	0x00000100
#define PG_kmemcg	0x00000200
#define PG_pte		0x00000400
... etc ...

I think that's a worthwhile improvement, particularly since I think
PG_kmemcg might want to be a completely independent flag from everything
else ("This is a page allocated for userspace PTEs, so it's both a kmemcg
page and a PTE page").

I'll respin the patch.  Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] mm: Split page_type out from _map_count
  2018-02-09 13:49   ` Matthew Wilcox
@ 2018-02-09 15:28     ` Matthew Wilcox
  2018-02-09 18:43       ` Dave Hansen
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Wilcox @ 2018-02-09 15:28 UTC (permalink / raw)
  To: Kirill A. Shutemov; +Cc: linux-mm, Matthew Wilcox

From: Matthew Wilcox <mawilcox@microsoft.com>

We're already using a union of many fields here, so stop abusing the
_map_count and make page_type its own field.  That implies renaming some
of the machinery that creates PageBuddy, PageBalloon and PageKmemcg;
bring back the PG_buddy, PG_balloon and PG_kmemcg names.  As Kirill
pointed out, it would be nice to be able to set more than one flag
in this field, so change the definitions to allow this.

Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
v2: Fixed a few places that still referred to the old MAPCOUNT_VALUEs.
    Incorporated Kirill's suggestion to allow more than one flag.

 include/linux/mm_types.h   | 13 ++++++++-----
 include/linux/page-flags.h | 44 +++++++++++++++++++++++++-------------------
 kernel/crash_core.c        |  2 +-
 mm/page_alloc.c            | 13 +++++--------
 scripts/tags.sh            |  6 +++---
 5 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index fd1af6b9591d..1c5dea402501 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -94,6 +94,14 @@ struct page {
 	};
 
 	union {
+		/*
+		 * If the page is neither PageSlab nor PageAnon, the value
+		 * stored here may help distinguish it from page cache pages.
+		 * See page-flags.h for a list of page types which are
+		 * currently stored here.
+		 */
+		unsigned int page_type;
+
 		_slub_counter_t counters;
 		unsigned int active;		/* SLAB */
 		struct {			/* SLUB */
@@ -107,11 +115,6 @@ struct page {
 			/*
 			 * Count of ptes mapped in mms, to show when
 			 * page is mapped & limit reverse map searches.
-			 *
-			 * Extra information about page type may be
-			 * stored here for pages that are never mapped,
-			 * in which case the value MUST BE <= -2.
-			 * See page-flags.h for more details.
 			 */
 			atomic_t _mapcount;
 
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 50c2b8786831..efe5ebfef5b2 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -630,49 +630,55 @@ PAGEFLAG_FALSE(DoubleMap)
 #endif
 
 /*
- * For pages that are never mapped to userspace, page->mapcount may be
- * used for storing extra information about page type. Any value used
- * for this purpose must be <= -2, but it's better start not too close
- * to -2 so that an underflow of the page_mapcount() won't be mistaken
- * for a special page.
+ * For pages that are never mapped to userspace (and aren't PageSlab),
+ * page_type may be used.  Because it is initialised to -1, we invert the
+ * sense of the bit, so SetPageFoo *clears* the bit used for PageFoo, and
+ * ClearPageFoo *sets* the bit used for PageFoo.  We leave a gap in the bit
+ * assignments so that an underflow of page_mapcount() won't be mistaken for
+ * a special page.
  */
-#define PAGE_MAPCOUNT_OPS(uname, lname)					\
+
+#define PAGE_TYPE_BASE	0xfffff000
+#define PG_buddy	0x00000080
+#define PG_balloon	0x00000100
+#define PG_kmemcg	0x00000200
+
+#define __is_page_type(page, flag)					\
+	((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)
+
+#define PAGE_TYPE_OPS(uname, lname)					\
 static __always_inline int Page##uname(struct page *page)		\
 {									\
-	return atomic_read(&page->_mapcount) ==				\
-				PAGE_##lname##_MAPCOUNT_VALUE;		\
+	return __is_page_type(page, PG_##lname);			\
 }									\
 static __always_inline void __SetPage##uname(struct page *page)		\
 {									\
-	VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);	\
-	atomic_set(&page->_mapcount, PAGE_##lname##_MAPCOUNT_VALUE);	\
+	VM_BUG_ON_PAGE(!__is_page_type(page, 0), page);			\
+	page->page_type &= ~PG_##lname;					\
 }									\
 static __always_inline void __ClearPage##uname(struct page *page)	\
 {									\
 	VM_BUG_ON_PAGE(!Page##uname(page), page);			\
-	atomic_set(&page->_mapcount, -1);				\
+	page->page_type |= PG_##lname;					\
 }
 
 /*
- * PageBuddy() indicate that the page is free and in the buddy system
+ * PageBuddy() indicates that the page is free and in the buddy system
  * (see mm/page_alloc.c).
  */
-#define PAGE_BUDDY_MAPCOUNT_VALUE		(-128)
-PAGE_MAPCOUNT_OPS(Buddy, BUDDY)
+PAGE_TYPE_OPS(Buddy, buddy)
 
 /*
- * PageBalloon() is set on pages that are on the balloon page list
+ * PageBalloon() is true for pages that are on the balloon page list
  * (see mm/balloon_compaction.c).
  */
-#define PAGE_BALLOON_MAPCOUNT_VALUE		(-256)
-PAGE_MAPCOUNT_OPS(Balloon, BALLOON)
+PAGE_TYPE_OPS(Balloon, balloon)
 
 /*
  * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
  * pages allocated with __GFP_ACCOUNT. It gets cleared on page free.
  */
-#define PAGE_KMEMCG_MAPCOUNT_VALUE		(-512)
-PAGE_MAPCOUNT_OPS(Kmemcg, KMEMCG)
+PAGE_TYPE_OPS(Kmemcg, kmemcg)
 
 extern bool is_free_buddy_page(struct page *page);
 
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 4f63597c824d..490760ada638 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -458,7 +458,7 @@ static int __init crash_save_vmcoreinfo_init(void)
 	VMCOREINFO_NUMBER(PG_hwpoison);
 #endif
 	VMCOREINFO_NUMBER(PG_head_mask);
-	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
+	VMCOREINFO_NUMBER(PG_buddy);
 #ifdef CONFIG_HUGETLB_PAGE
 	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
 #endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 81e18ceef579..ef9c259db041 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -740,16 +740,14 @@ static inline void rmv_page_order(struct page *page)
 
 /*
  * This function checks whether a page is free && is the buddy
- * we can do coalesce a page and its buddy if
+ * we can coalesce a page and its buddy if
  * (a) the buddy is not in a hole (check before calling!) &&
  * (b) the buddy is in the buddy system &&
  * (c) a page and its buddy have the same order &&
  * (d) a page and its buddy are in the same zone.
  *
- * For recording whether a page is in the buddy system, we set ->_mapcount
- * PAGE_BUDDY_MAPCOUNT_VALUE.
- * Setting, clearing, and testing _mapcount PAGE_BUDDY_MAPCOUNT_VALUE is
- * serialized by zone->lock.
+ * For recording whether a page is in the buddy system, we set PG_buddy.
+ * Setting, clearing, and testing PG_buddy is serialized by zone->lock.
  *
  * For recording page's order, we use page_private(page).
  */
@@ -794,9 +792,8 @@ static inline int page_is_buddy(struct page *page, struct page *buddy,
  * as necessary, plus some accounting needed to play nicely with other
  * parts of the VM system.
  * At each level, we keep a list of pages, which are heads of continuous
- * free pages of length of (1 << order) and marked with _mapcount
- * PAGE_BUDDY_MAPCOUNT_VALUE. Page's order is recorded in page_private(page)
- * field.
+ * free pages of length of (1 << order) and marked with PageBuddy().
+ * Page's order is recorded in page_private(page) field.
  * So when we are allocating or freeing one, we can derive the state of the
  * other.  That is, if we allocate a small block, and both were
  * free, the remainder of the region must be split into blocks.
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c..8c3ae36d4ea8 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -188,9 +188,9 @@ regex_c=(
 	'/\<CLEARPAGEFLAG_NOOP(\([[:alnum:]_]*\).*/ClearPage\1/'
 	'/\<__CLEARPAGEFLAG_NOOP(\([[:alnum:]_]*\).*/__ClearPage\1/'
 	'/\<TESTCLEARFLAG_FALSE(\([[:alnum:]_]*\).*/TestClearPage\1/'
-	'/^PAGE_MAPCOUNT_OPS(\([[:alnum:]_]*\).*/Page\1/'
-	'/^PAGE_MAPCOUNT_OPS(\([[:alnum:]_]*\).*/__SetPage\1/'
-	'/^PAGE_MAPCOUNT_OPS(\([[:alnum:]_]*\).*/__ClearPage\1/'
+	'/^PAGE_TYPE_OPS(\([[:alnum:]_]*\).*/Page\1/'
+	'/^PAGE_TYPE_OPS(\([[:alnum:]_]*\).*/__SetPage\1/'
+	'/^PAGE_TYPE_OPS(\([[:alnum:]_]*\).*/__ClearPage\1/'
 	'/^TASK_PFA_TEST([^,]*, *\([[:alnum:]_]*\))/task_\1/'
 	'/^TASK_PFA_SET([^,]*, *\([[:alnum:]_]*\))/task_set_\1/'
 	'/^TASK_PFA_CLEAR([^,]*, *\([[:alnum:]_]*\))/task_clear_\1/'
-- 
2.15.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mm: Split page_type out from _map_count
  2018-02-09 15:28     ` [PATCH v2] " Matthew Wilcox
@ 2018-02-09 18:43       ` Dave Hansen
  2018-02-09 19:04         ` Christopher Lameter
  2018-02-09 19:28         ` Matthew Wilcox
  0 siblings, 2 replies; 8+ messages in thread
From: Dave Hansen @ 2018-02-09 18:43 UTC (permalink / raw)
  To: Matthew Wilcox, Kirill A. Shutemov; +Cc: linux-mm, Matthew Wilcox

On 02/09/2018 07:28 AM, Matthew Wilcox wrote:
>  	union {
> +		/*
> +		 * If the page is neither PageSlab nor PageAnon, the value
> +		 * stored here may help distinguish it from page cache pages.
> +		 * See page-flags.h for a list of page types which are
> +		 * currently stored here.
> +		 */
> +		unsigned int page_type;
> +
>  		_slub_counter_t counters;
>  		unsigned int active;		/* SLAB */
>  		struct {			/* SLUB */
> @@ -107,11 +115,6 @@ struct page {
>  			/*
>  			 * Count of ptes mapped in mms, to show when
>  			 * page is mapped & limit reverse map searches.
> -			 *
> -			 * Extra information about page type may be
> -			 * stored here for pages that are never mapped,
> -			 * in which case the value MUST BE <= -2.
> -			 * See page-flags.h for more details.
>  			 */
>  			atomic_t _mapcount;

Are there any straightforward rules that we can enforce here?  For
instance, if you are using "page_type", you can never have PG_lru set.

Not that we have done this at all for 'struct page' historically, it
would be really convenient to have a clear definition for when
"page_type" is valid vs. "_mapcount".

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mm: Split page_type out from _map_count
  2018-02-09 18:43       ` Dave Hansen
@ 2018-02-09 19:04         ` Christopher Lameter
  2018-02-09 19:28         ` Matthew Wilcox
  1 sibling, 0 replies; 8+ messages in thread
From: Christopher Lameter @ 2018-02-09 19:04 UTC (permalink / raw)
  To: Dave Hansen; +Cc: Matthew Wilcox, Kirill A. Shutemov, linux-mm, Matthew Wilcox

On Fri, 9 Feb 2018, Dave Hansen wrote:

> Are there any straightforward rules that we can enforce here?  For
> instance, if you are using "page_type", you can never have PG_lru set.
>
> Not that we have done this at all for 'struct page' historically, it
> would be really convenient to have a clear definition for when
> "page_type" is valid vs. "_mapcount".

Well in general we would like to be able to enforce uses depending on
the contents of other fields in struct page. That would require compiler
support I guess?

What we could do is write a struct page validator that checks contents
using some macros? Could be added to the usual places where we check
consistency and could also be used for a global sweep over struct pages
for validation.

SLUB can do that for metadata. If we could express consistency rules for
objects in general then it may even have a wider applicability.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mm: Split page_type out from _map_count
  2018-02-09 18:43       ` Dave Hansen
  2018-02-09 19:04         ` Christopher Lameter
@ 2018-02-09 19:28         ` Matthew Wilcox
  1 sibling, 0 replies; 8+ messages in thread
From: Matthew Wilcox @ 2018-02-09 19:28 UTC (permalink / raw)
  To: Dave Hansen; +Cc: Kirill A. Shutemov, linux-mm, Matthew Wilcox

On Fri, Feb 09, 2018 at 10:43:19AM -0800, Dave Hansen wrote:
> On 02/09/2018 07:28 AM, Matthew Wilcox wrote:
> >  	union {
> > +		/*
> > +		 * If the page is neither PageSlab nor PageAnon, the value
> > +		 * stored here may help distinguish it from page cache pages.
> > +		 * See page-flags.h for a list of page types which are
> > +		 * currently stored here.
> > +		 */
> > +		unsigned int page_type;
> > +
> >  		_slub_counter_t counters;
> >  		unsigned int active;		/* SLAB */
> >  		struct {			/* SLUB */
> 
> Are there any straightforward rules that we can enforce here?  For
> instance, if you are using "page_type", you can never have PG_lru set.
> 
> Not that we have done this at all for 'struct page' historically, it
> would be really convenient to have a clear definition for when
> "page_type" is valid vs. "_mapcount".

I agree, it'd be nice.  I think the only invariant we can claim to be
true is that if PageSlab is set then page_type is not valid.  There are
probably any number of bits in the page->flags that aren't currently in
use by any of the consumers who actually set page_type, but I don't feel
like there's a straightforward rule that we can enforce.  Maybe they'll
want to start using them in the future for their own purposes.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: Split page_type out from _map_count
  2018-02-07 21:30 [PATCH] mm: Split page_type out from _map_count Matthew Wilcox
  2018-02-09 10:51 ` Kirill A. Shutemov
@ 2018-02-10  5:00 ` kbuild test robot
  1 sibling, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2018-02-10  5:00 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: kbuild-all, linux-mm, Matthew Wilcox

[-- Attachment #1: Type: text/plain, Size: 9090 bytes --]

Hi Matthew,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on next-20180209]
[cannot apply to v4.15]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox/mm-Split-page_type-out-from-_map_count/20180210-114226
config: i386-randconfig-s1-201805 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from kernel/crash_core.c:9:0:
   kernel/crash_core.c: In function 'crash_save_vmcoreinfo_init':
>> kernel/crash_core.c:461:20: error: 'PAGE_BUDDY_MAPCOUNT_VALUE' undeclared (first use in this function)
     VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
                       ^
   include/linux/crash_core.h:59:57: note: in definition of macro 'VMCOREINFO_NUMBER'
     vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
                                                            ^~~~
   kernel/crash_core.c:461:20: note: each undeclared identifier is reported only once for each function it appears in
     VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
                       ^
   include/linux/crash_core.h:59:57: note: in definition of macro 'VMCOREINFO_NUMBER'
     vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
                                                            ^~~~

vim +/PAGE_BUDDY_MAPCOUNT_VALUE +461 kernel/crash_core.c

692f66f2 Hari Bathini       2017-05-08  379  
692f66f2 Hari Bathini       2017-05-08  380  static int __init crash_save_vmcoreinfo_init(void)
692f66f2 Hari Bathini       2017-05-08  381  {
203e9e41 Xunlei Pang        2017-07-12  382  	vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL);
203e9e41 Xunlei Pang        2017-07-12  383  	if (!vmcoreinfo_data) {
203e9e41 Xunlei Pang        2017-07-12  384  		pr_warn("Memory allocation for vmcoreinfo_data failed\n");
203e9e41 Xunlei Pang        2017-07-12  385  		return -ENOMEM;
203e9e41 Xunlei Pang        2017-07-12  386  	}
203e9e41 Xunlei Pang        2017-07-12  387  
203e9e41 Xunlei Pang        2017-07-12  388  	vmcoreinfo_note = alloc_pages_exact(VMCOREINFO_NOTE_SIZE,
203e9e41 Xunlei Pang        2017-07-12  389  						GFP_KERNEL | __GFP_ZERO);
203e9e41 Xunlei Pang        2017-07-12  390  	if (!vmcoreinfo_note) {
203e9e41 Xunlei Pang        2017-07-12  391  		free_page((unsigned long)vmcoreinfo_data);
203e9e41 Xunlei Pang        2017-07-12  392  		vmcoreinfo_data = NULL;
203e9e41 Xunlei Pang        2017-07-12  393  		pr_warn("Memory allocation for vmcoreinfo_note failed\n");
203e9e41 Xunlei Pang        2017-07-12  394  		return -ENOMEM;
203e9e41 Xunlei Pang        2017-07-12  395  	}
203e9e41 Xunlei Pang        2017-07-12  396  
692f66f2 Hari Bathini       2017-05-08  397  	VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
692f66f2 Hari Bathini       2017-05-08  398  	VMCOREINFO_PAGESIZE(PAGE_SIZE);
692f66f2 Hari Bathini       2017-05-08  399  
692f66f2 Hari Bathini       2017-05-08  400  	VMCOREINFO_SYMBOL(init_uts_ns);
692f66f2 Hari Bathini       2017-05-08  401  	VMCOREINFO_SYMBOL(node_online_map);
692f66f2 Hari Bathini       2017-05-08  402  #ifdef CONFIG_MMU
692f66f2 Hari Bathini       2017-05-08  403  	VMCOREINFO_SYMBOL(swapper_pg_dir);
692f66f2 Hari Bathini       2017-05-08  404  #endif
692f66f2 Hari Bathini       2017-05-08  405  	VMCOREINFO_SYMBOL(_stext);
692f66f2 Hari Bathini       2017-05-08  406  	VMCOREINFO_SYMBOL(vmap_area_list);
692f66f2 Hari Bathini       2017-05-08  407  
692f66f2 Hari Bathini       2017-05-08  408  #ifndef CONFIG_NEED_MULTIPLE_NODES
692f66f2 Hari Bathini       2017-05-08  409  	VMCOREINFO_SYMBOL(mem_map);
692f66f2 Hari Bathini       2017-05-08  410  	VMCOREINFO_SYMBOL(contig_page_data);
692f66f2 Hari Bathini       2017-05-08  411  #endif
692f66f2 Hari Bathini       2017-05-08  412  #ifdef CONFIG_SPARSEMEM
a0b12803 Kirill A. Shutemov 2018-01-12  413  	VMCOREINFO_SYMBOL_ARRAY(mem_section);
692f66f2 Hari Bathini       2017-05-08  414  	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
692f66f2 Hari Bathini       2017-05-08  415  	VMCOREINFO_STRUCT_SIZE(mem_section);
692f66f2 Hari Bathini       2017-05-08  416  	VMCOREINFO_OFFSET(mem_section, section_mem_map);
692f66f2 Hari Bathini       2017-05-08  417  #endif
692f66f2 Hari Bathini       2017-05-08  418  	VMCOREINFO_STRUCT_SIZE(page);
692f66f2 Hari Bathini       2017-05-08  419  	VMCOREINFO_STRUCT_SIZE(pglist_data);
692f66f2 Hari Bathini       2017-05-08  420  	VMCOREINFO_STRUCT_SIZE(zone);
692f66f2 Hari Bathini       2017-05-08  421  	VMCOREINFO_STRUCT_SIZE(free_area);
692f66f2 Hari Bathini       2017-05-08  422  	VMCOREINFO_STRUCT_SIZE(list_head);
692f66f2 Hari Bathini       2017-05-08  423  	VMCOREINFO_SIZE(nodemask_t);
692f66f2 Hari Bathini       2017-05-08  424  	VMCOREINFO_OFFSET(page, flags);
692f66f2 Hari Bathini       2017-05-08  425  	VMCOREINFO_OFFSET(page, _refcount);
692f66f2 Hari Bathini       2017-05-08  426  	VMCOREINFO_OFFSET(page, mapping);
692f66f2 Hari Bathini       2017-05-08  427  	VMCOREINFO_OFFSET(page, lru);
692f66f2 Hari Bathini       2017-05-08  428  	VMCOREINFO_OFFSET(page, _mapcount);
692f66f2 Hari Bathini       2017-05-08  429  	VMCOREINFO_OFFSET(page, private);
692f66f2 Hari Bathini       2017-05-08  430  	VMCOREINFO_OFFSET(page, compound_dtor);
692f66f2 Hari Bathini       2017-05-08  431  	VMCOREINFO_OFFSET(page, compound_order);
692f66f2 Hari Bathini       2017-05-08  432  	VMCOREINFO_OFFSET(page, compound_head);
692f66f2 Hari Bathini       2017-05-08  433  	VMCOREINFO_OFFSET(pglist_data, node_zones);
692f66f2 Hari Bathini       2017-05-08  434  	VMCOREINFO_OFFSET(pglist_data, nr_zones);
692f66f2 Hari Bathini       2017-05-08  435  #ifdef CONFIG_FLAT_NODE_MEM_MAP
692f66f2 Hari Bathini       2017-05-08  436  	VMCOREINFO_OFFSET(pglist_data, node_mem_map);
692f66f2 Hari Bathini       2017-05-08  437  #endif
692f66f2 Hari Bathini       2017-05-08  438  	VMCOREINFO_OFFSET(pglist_data, node_start_pfn);
692f66f2 Hari Bathini       2017-05-08  439  	VMCOREINFO_OFFSET(pglist_data, node_spanned_pages);
692f66f2 Hari Bathini       2017-05-08  440  	VMCOREINFO_OFFSET(pglist_data, node_id);
692f66f2 Hari Bathini       2017-05-08  441  	VMCOREINFO_OFFSET(zone, free_area);
692f66f2 Hari Bathini       2017-05-08  442  	VMCOREINFO_OFFSET(zone, vm_stat);
692f66f2 Hari Bathini       2017-05-08  443  	VMCOREINFO_OFFSET(zone, spanned_pages);
692f66f2 Hari Bathini       2017-05-08  444  	VMCOREINFO_OFFSET(free_area, free_list);
692f66f2 Hari Bathini       2017-05-08  445  	VMCOREINFO_OFFSET(list_head, next);
692f66f2 Hari Bathini       2017-05-08  446  	VMCOREINFO_OFFSET(list_head, prev);
692f66f2 Hari Bathini       2017-05-08  447  	VMCOREINFO_OFFSET(vmap_area, va_start);
692f66f2 Hari Bathini       2017-05-08  448  	VMCOREINFO_OFFSET(vmap_area, list);
692f66f2 Hari Bathini       2017-05-08  449  	VMCOREINFO_LENGTH(zone.free_area, MAX_ORDER);
692f66f2 Hari Bathini       2017-05-08  450  	log_buf_vmcoreinfo_setup();
692f66f2 Hari Bathini       2017-05-08  451  	VMCOREINFO_LENGTH(free_area.free_list, MIGRATE_TYPES);
692f66f2 Hari Bathini       2017-05-08  452  	VMCOREINFO_NUMBER(NR_FREE_PAGES);
692f66f2 Hari Bathini       2017-05-08  453  	VMCOREINFO_NUMBER(PG_lru);
692f66f2 Hari Bathini       2017-05-08  454  	VMCOREINFO_NUMBER(PG_private);
692f66f2 Hari Bathini       2017-05-08  455  	VMCOREINFO_NUMBER(PG_swapcache);
692f66f2 Hari Bathini       2017-05-08  456  	VMCOREINFO_NUMBER(PG_slab);
692f66f2 Hari Bathini       2017-05-08  457  #ifdef CONFIG_MEMORY_FAILURE
692f66f2 Hari Bathini       2017-05-08  458  	VMCOREINFO_NUMBER(PG_hwpoison);
692f66f2 Hari Bathini       2017-05-08  459  #endif
692f66f2 Hari Bathini       2017-05-08  460  	VMCOREINFO_NUMBER(PG_head_mask);
692f66f2 Hari Bathini       2017-05-08 @461  	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
692f66f2 Hari Bathini       2017-05-08  462  #ifdef CONFIG_HUGETLB_PAGE
692f66f2 Hari Bathini       2017-05-08  463  	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
692f66f2 Hari Bathini       2017-05-08  464  #endif
692f66f2 Hari Bathini       2017-05-08  465  
692f66f2 Hari Bathini       2017-05-08  466  	arch_crash_save_vmcoreinfo();
692f66f2 Hari Bathini       2017-05-08  467  	update_vmcoreinfo_note();
692f66f2 Hari Bathini       2017-05-08  468  
692f66f2 Hari Bathini       2017-05-08  469  	return 0;
692f66f2 Hari Bathini       2017-05-08  470  }
692f66f2 Hari Bathini       2017-05-08  471  

:::::: The code at line 461 was first introduced by commit
:::::: 692f66f26a4c19d73249736aa973c13a1521b387 crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE

:::::: TO: Hari Bathini <hbathini@linux.vnet.ibm.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32428 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-02-10  5:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 21:30 [PATCH] mm: Split page_type out from _map_count Matthew Wilcox
2018-02-09 10:51 ` Kirill A. Shutemov
2018-02-09 13:49   ` Matthew Wilcox
2018-02-09 15:28     ` [PATCH v2] " Matthew Wilcox
2018-02-09 18:43       ` Dave Hansen
2018-02-09 19:04         ` Christopher Lameter
2018-02-09 19:28         ` Matthew Wilcox
2018-02-10  5:00 ` [PATCH] " kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.