[PATCH] mm: Improve dump_page() for compound pages

[PATCH] mm: Improve dump_page() for compound pages

[Matthew Wilcox]

From: "Matthew Wilcox (Oracle)" <willy@infradead.org>

There was no protection against a corrupted struct page having an
implausible compound_head().  Sanity check that a compound page has
a head within reach of the maximum allocatable page (this will need
to be adjusted if one of the plans to allocate 1GB pages comes to
fruition).  In addition,

 - Print the mapping pointer using %p insted of %px.  The actual value of
   the pointer can be read out of the raw page dump and using %p gives a
   chance to correlate it with an earlier printk of the mapping pointer
 - Print the mapping pointer from the head page, not the tail page
   (the tail ->mapping pointer may be in use for other purposes, eg part
   of a list_head)
 - Print the order of the page for compound pages
 - Dump the raw head page as well as the raw page
 - Print the refcount from the head page, not the tail page

Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Co-developed-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/debug.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/mm/debug.c b/mm/debug.c
index ecccd9f17801..3594951cc408 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -44,8 +44,10 @@ const struct trace_print_flags vmaflag_names[] = {
 
 void __dump_page(struct page *page, const char *reason)
 {
+	struct page *head = compound_head(page);
 	struct address_space *mapping;
 	bool page_poisoned = PagePoisoned(page);
+	bool compound = PageCompound(page);
 	/*
 	 * Accessing the pageblock without the zone lock. It could change to
 	 * "isolate" again in the meantime, but since we are just dumping the
@@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason)
 		goto hex_only;
 	}
 
-	mapping = page_mapping(page);
+	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
+		/* Corrupt page, cannot call page_mapping */
+		mapping = page->mapping;
+		head = page;
+		compound = false;
+	} else {
+		mapping = page_mapping(page);
+	}
 
 	/*
 	 * Avoid VM_BUG_ON() in page_mapcount().
 	 * page->_mapcount space in struct page is used by sl[aou]b pages to
 	 * encode own info.
 	 */
-	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
+	mapcount = PageSlab(head) ? 0 : page_mapcount(head);
 
-	if (PageCompound(page))
-		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px "
-			"index:%#lx compound_mapcount: %d\n",
-			page, page_ref_count(page), mapcount,
-			page->mapping, page_to_pgoff(page),
-			compound_mapcount(page));
+	if (compound)
+		pr_warn("page:%px refcount:%d mapcount:%d mapping:%p "
+			"index:%#lx head:%px order:%u compound_mapcount:%d\n",
+			page, page_ref_count(head), mapcount,
+			mapping, page_to_pgoff(page), head,
+			compound_order(head), compound_mapcount(page));
 	else
-		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
+		pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n",
 			page, page_ref_count(page), mapcount,
-			page->mapping, page_to_pgoff(page));
+			mapping, page_to_pgoff(page));
 	if (PageKsm(page))
 		type = "ksm ";
 	else if (PageAnon(page))
@@ -106,6 +115,10 @@ void __dump_page(struct page *page, const char *reason)
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
 			sizeof(struct page), false);
+	if (head != page)
+		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
+			sizeof(unsigned long), head,
+			sizeof(struct page), false);
 
 	if (reason)
 		pr_warn("page dumped because: %s\n", reason);
-- 
2.24.1

Re: [PATCH] mm: Improve dump_page() for compound pages

[Matthew Wilcox]

On Fri, Feb 07, 2020 at 08:44:15PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> 
> There was no protection against a corrupted struct page having an
> implausible compound_head().  Sanity check that a compound page has
> a head within reach of the maximum allocatable page (this will need
> to be adjusted if one of the plans to allocate 1GB pages comes to
> fruition).  In addition,
> 
>  - Print the mapping pointer using %p insted of %px.  The actual value of
>    the pointer can be read out of the raw page dump and using %p gives a
>    chance to correlate it with an earlier printk of the mapping pointer
>  - Print the mapping pointer from the head page, not the tail page
>    (the tail ->mapping pointer may be in use for other purposes, eg part
>    of a list_head)
>  - Print the order of the page for compound pages
>  - Dump the raw head page as well as the raw page
>  - Print the refcount from the head page, not the tail page
> 
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

John, in comparison to the patch I sent earlier today, this version changes:

 - Reorder the things printed in the compound case so that all the information
   which is printed for non-compound pages is printed first
 - Removed inconsistent space between "compound_mapcount:" and the value
 - Print the refcount of the head page instead of the tail (which was the
   point of your patch!)
 - Print the mapping of the head instead of the tail
 - Don't dump the raw head if the page passed in was the head page

Re: [PATCH] mm: Improve dump_page() for compound pages

[John Hubbard]

On 2/7/20 8:48 PM, Matthew Wilcox wrote:
> On Fri, Feb 07, 2020 at 08:44:15PM -0800, Matthew Wilcox wrote:
>> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>>
>> There was no protection against a corrupted struct page having an
>> implausible compound_head().  Sanity check that a compound page has
>> a head within reach of the maximum allocatable page (this will need
>> to be adjusted if one of the plans to allocate 1GB pages comes to
>> fruition).  In addition,
>>
>>  - Print the mapping pointer using %p insted of %px.  The actual value of
>>    the pointer can be read out of the raw page dump and using %p gives a
>>    chance to correlate it with an earlier printk of the mapping pointer
>>  - Print the mapping pointer from the head page, not the tail page
>>    (the tail ->mapping pointer may be in use for other purposes, eg part
>>    of a list_head)
>>  - Print the order of the page for compound pages
>>  - Dump the raw head page as well as the raw page
>>  - Print the refcount from the head page, not the tail page
>>
>> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Co-developed-by: John Hubbard <jhubbard@nvidia.com>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> John, in comparison to the patch I sent earlier today, this version changes:
> 
>  - Reorder the things printed in the compound case so that all the information
>    which is printed for non-compound pages is printed first
>  - Removed inconsistent space between "compound_mapcount:" and the value
>  - Print the refcount of the head page instead of the tail (which was the
>    point of your patch!)
>  - Print the mapping of the head instead of the tail
>  - Don't dump the raw head if the page passed in was the head page
> 

Got it, I've snarfed it into my series, thanks very much!


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] mm: Improve dump_page() for compound pages

[Matthew Wilcox]

On Sat, Feb 08, 2020 at 05:09:15PM -0800, John Hubbard wrote:
> Got it, I've snarfed it into my series, thanks very much!

Thank you!  I found the current debugging inadequate, but I'd only hacked
up what I needed for my own needs, and really debugging code should be
useful for situations we've not encountered yet.

Re: [PATCH] mm: Improve dump_page() for compound pages

[John Hubbard]

On 2/7/20 8:44 PM, Matthew Wilcox wrote:
...  
> -	if (PageCompound(page))
> -		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px "
> -			"index:%#lx compound_mapcount: %d\n",
> -			page, page_ref_count(page), mapcount,
> -			page->mapping, page_to_pgoff(page),
> -			compound_mapcount(page));
> +	if (compound)
> +		pr_warn("page:%px refcount:%d mapcount:%d mapping:%p "
> +			"index:%#lx head:%px order:%u compound_mapcount:%d\n",
> +			page, page_ref_count(head), mapcount,
> +			mapping, page_to_pgoff(page), head,
> +			compound_order(head), compound_mapcount(page));


OK, so the patch now produces the following output for a normal page and a huge page:

page:ffffea0011f68540 refcount:1025 mapcount:1 mapping:00000000b9ef1410 index:0x0
anon flags: 0x17ffe0000080036(referenced|uptodate|lru|active|swapbacked)
raw: 017ffe0000080036 ffffea0011f684c8 ffffea0010eeab88 ffff888495396581
raw: 0000000000000000 0000000000000000 0000040100000000 0000000000000000
page dumped because: test: dump_page()

page:ffffea0010ed2740 refcount:513 mapcount:1 mapping:00000000b9ef1410 index:0xb2 head:ffffea0010ed0000 order:9 compound_mapcount:1
anon flags: 0x17ffe0000000000()
raw: 017ffe0000000000 ffffea0010ed0001 ffffea0010ed2748 dead000000000400
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
head: 017ffe0000090036 ffffea0011ff8ec8 ffffea0010ed8008 ffff888495396581
head: 0000000000000015 0000000000000000 00000201ffffffff 0000000000000000
page dumped because: test: dump_page()

...which is looking very good!

Minor point: How do you and everyone feel about the line length? I do see it getting 
pretty long, and I'm about to add the following to it, as a separate patch on top of this:

    "compound_pincount:N"

...which just makes it even worse. And on some serial terminals this can get cut
off, so I'm wondering if maybe doing another pr_warn(), and a duplicated page pointer
output, to break up the line would help, like this output:


page:ffffea0010ed2740 refcount:513 mapcount:1 mapping:00000000b9ef1410 index:0xb2 head:ffffea0010ed0000 
page:ffffea0010ed2740 order:9 compound_mapcount:1
anon flags: 0x17ffe0000000000()
raw: 017ffe0000000000 ffffea0010ed0001 ffffea0010ed2748 dead000000000400
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
head: 017ffe0000090036 ffffea0011ff8ec8 ffffea0010ed8008 ffff888495396581
head: 0000000000000015 0000000000000000 00000201ffffffff 0000000000000000
page dumped because: test: dump_page()

...which then becomes this, in a future patch:

page:ffffea0010ed2740 refcount:513 mapcount:1 mapping:00000000b9ef1410 index:0xb2 head:ffffea0010ed0000 
page:ffffea0010ed2740 order:9 compound_mapcount:1 compound_pincount:1
anon flags: 0x17ffe0000000000()
raw: 017ffe0000000000 ffffea0010ed0001 ffffea0010ed2748 dead000000000400
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
head: 017ffe0000090036 ffffea0011ff8ec8 ffffea0010ed8008 ffff888495396581
head: 0000000000000015 0000000000000000 00000201ffffffff 0000000000000000
page dumped because: test: dump_page()


...or is it best in your experience to leave that line as an "atomic" print statement?


thanks,
-- 
John Hubbard
NVIDIA

>  	else
> -		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
> +		pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n",
>  			page, page_ref_count(page), mapcount,
> -			page->mapping, page_to_pgoff(page));
> +			mapping, page_to_pgoff(page));
>  	if (PageKsm(page))
>  		type = "ksm ";
>  	else if (PageAnon(page))
> @@ -106,6 +115,10 @@ void __dump_page(struct page *page, const char *reason)
>  	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>  			sizeof(unsigned long), page,
>  			sizeof(struct page), false);
> +	if (head != page)
> +		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
> +			sizeof(unsigned long), head,
> +			sizeof(struct page), false);
>  
>  	if (reason)
>  		pr_warn("page dumped because: %s\n", reason);
> 

Re: [PATCH] mm: Improve dump_page() for compound pages

[Kirill A. Shutemov]

On Fri, Feb 07, 2020 at 08:44:15PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
> 
> There was no protection against a corrupted struct page having an
> implausible compound_head().  Sanity check that a compound page has
> a head within reach of the maximum allocatable page (this will need
> to be adjusted if one of the plans to allocate 1GB pages comes to
> fruition).  In addition,
> 
>  - Print the mapping pointer using %p insted of %px.  The actual value of
>    the pointer can be read out of the raw page dump and using %p gives a
>    chance to correlate it with an earlier printk of the mapping pointer
>  - Print the mapping pointer from the head page, not the tail page
>    (the tail ->mapping pointer may be in use for other purposes, eg part
>    of a list_head)
>  - Print the order of the page for compound pages
>  - Dump the raw head page as well as the raw page
>  - Print the refcount from the head page, not the tail page
> 
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/debug.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/debug.c b/mm/debug.c
> index ecccd9f17801..3594951cc408 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -44,8 +44,10 @@ const struct trace_print_flags vmaflag_names[] = {
>  
>  void __dump_page(struct page *page, const char *reason)
>  {
> +	struct page *head = compound_head(page);
>  	struct address_space *mapping;
>  	bool page_poisoned = PagePoisoned(page);
> +	bool compound = PageCompound(page);
>  	/*
>  	 * Accessing the pageblock without the zone lock. It could change to
>  	 * "isolate" again in the meantime, but since we are just dumping the
> @@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason)
>  		goto hex_only;
>  	}
>  
> -	mapping = page_mapping(page);
> +	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
> +		/* Corrupt page, cannot call page_mapping */
> +		mapping = page->mapping;
> +		head = page;
> +		compound = false;
> +	} else {
> +		mapping = page_mapping(page);
> +	}
>  
>  	/*
>  	 * Avoid VM_BUG_ON() in page_mapcount().
>  	 * page->_mapcount space in struct page is used by sl[aou]b pages to
>  	 * encode own info.
>  	 */
> -	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
> +	mapcount = PageSlab(head) ? 0 : page_mapcount(head);

This is wrong. We want to see mapcount for the tail page, not head.

-- 
 Kirill A. Shutemov

Re: [PATCH] mm: Improve dump_page() for compound pages

[John Hubbard]

On 2/10/20 4:42 AM, Kirill A. Shutemov wrote:
...
>> @@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason)
>>  		goto hex_only;
>>  	}
>>  
>> -	mapping = page_mapping(page);
>> +	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
>> +		/* Corrupt page, cannot call page_mapping */
>> +		mapping = page->mapping;
>> +		head = page;
>> +		compound = false;
>> +	} else {
>> +		mapping = page_mapping(page);
>> +	}
>>  
>>  	/*
>>  	 * Avoid VM_BUG_ON() in page_mapcount().
>>  	 * page->_mapcount space in struct page is used by sl[aou]b pages to
>>  	 * encode own info.
>>  	 */
>> -	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
>> +	mapcount = PageSlab(head) ? 0 : page_mapcount(head);
> 
> This is wrong. We want to see mapcount for the tail page, not head.
> 

I see what you mean: page_mapcount(page) sums up both the page's and the
head page's mapcount in some cases. The function doesn't seem to work
correctly unless it is fed the tail page.

Here, even though the "head" variable's meaning is overloaded (=="head page, 
unless the tail page was corrupted, in which case, tail page"), it would still be
accurate to change that line back to the original line, so that it once again
reads:

        mapcount = PageSlab(page) ? 0 : page_mapcount(page);

Matthew?

(Also, I see that __page_mapcount is EXPORT-ed, which is odd: nothing uses it
other than page_mapcount. Micro-housecleaning time maybe...)


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] mm: Improve dump_page() for compound pages

[Kirill A. Shutemov]

On Mon, Feb 10, 2020 at 11:50:21AM -0800, John Hubbard wrote:
> On 2/10/20 4:42 AM, Kirill A. Shutemov wrote:
> ...
> >> @@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason)
> >>  		goto hex_only;
> >>  	}
> >>  
> >> -	mapping = page_mapping(page);
> >> +	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
> >> +		/* Corrupt page, cannot call page_mapping */
> >> +		mapping = page->mapping;
> >> +		head = page;
> >> +		compound = false;
> >> +	} else {
> >> +		mapping = page_mapping(page);
> >> +	}
> >>  
> >>  	/*
> >>  	 * Avoid VM_BUG_ON() in page_mapcount().
> >>  	 * page->_mapcount space in struct page is used by sl[aou]b pages to
> >>  	 * encode own info.
> >>  	 */
> >> -	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
> >> +	mapcount = PageSlab(head) ? 0 : page_mapcount(head);
> > 
> > This is wrong. We want to see mapcount for the tail page, not head.
> > 
> 
> I see what you mean: page_mapcount(page) sums up both the page's and the
> head page's mapcount in some cases. The function doesn't seem to work
> correctly unless it is fed the tail page.

What makes you think this? It has to be called on the page user provided.
Head or tail.

> Here, even though the "head" variable's meaning is overloaded (=="head page, 
> unless the tail page was corrupted, in which case, tail page"), it would still be
> accurate to change that line back to the original line, so that it once again
> reads:
> 
>         mapcount = PageSlab(page) ? 0 : page_mapcount(page);

PageSlab() can be called for the head page.

> 
> Matthew?
> 
> (Also, I see that __page_mapcount is EXPORT-ed, which is odd: nothing uses it
> other than page_mapcount.

... and page_mapcount() can be inlined anywhere.

-- 
 Kirill A. Shutemov

Re: [PATCH] mm: Improve dump_page() for compound pages

[John Hubbard]

On 2/10/20 1:21 PM, Kirill A. Shutemov wrote:
> On Mon, Feb 10, 2020 at 11:50:21AM -0800, John Hubbard wrote:
>> On 2/10/20 4:42 AM, Kirill A. Shutemov wrote:
>> ...
>>>> @@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason)
>>>>  		goto hex_only;
>>>>  	}
>>>>  
>>>> -	mapping = page_mapping(page);
>>>> +	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
>>>> +		/* Corrupt page, cannot call page_mapping */
>>>> +		mapping = page->mapping;
>>>> +		head = page;
>>>> +		compound = false;
>>>> +	} else {
>>>> +		mapping = page_mapping(page);
>>>> +	}
>>>>  
>>>>  	/*
>>>>  	 * Avoid VM_BUG_ON() in page_mapcount().
>>>>  	 * page->_mapcount space in struct page is used by sl[aou]b pages to
>>>>  	 * encode own info.
>>>>  	 */
>>>> -	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
>>>> +	mapcount = PageSlab(head) ? 0 : page_mapcount(head);
>>>
>>> This is wrong. We want to see mapcount for the tail page, not head.
>>>
>>
>> I see what you mean: page_mapcount(page) sums up both the page's and the
>> head page's mapcount in some cases. The function doesn't seem to work
>> correctly unless it is fed the tail page.
> 
> What makes you think this? It has to be called on the page user provided.
> Head or tail.


Actually, we are in total agreement there, but I meant "if it is a real
tail page"--in other words, I should have written:

"the function doesn't seem to work correctly unless it is fed the original 
page. If you feed it the head page unconditionally, it breaks 
page_mapcounts()'s assumptions". Sorry for the confusing response.


> 
>> Here, even though the "head" variable's meaning is overloaded (=="head page, 
>> unless the tail page was corrupted, in which case, tail page"), it would still be
>> accurate to change that line back to the original line, so that it once again
>> reads:
>>
>>         mapcount = PageSlab(page) ? 0 : page_mapcount(page);
> 
> PageSlab() can be called for the head page.


It's not clear to me whether it's safer to use the...user-provided page, or the
head page, for checking PageSlab()--it's probably set on both head and non-head
pages, right? I guess you're saying it should be like this:

	mapcount = PageSlab(head) ? 0 : page_mapcount(page);

...yes?

> 
>>
>> Matthew?
>>
>> (Also, I see that __page_mapcount is EXPORT-ed, which is odd: nothing uses it
>> other than page_mapcount.
> 
> ... and page_mapcount() can be inlined anywhere.
> 

sigh, please ignore anything I say about EXPORTS--I've been reminded of this point
before, I'm afraid. :)


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] mm: Improve dump_page() for compound pages

[Kirill A. Shutemov]

On Mon, Feb 10, 2020 at 01:33:22PM -0800, John Hubbard wrote:
> > PageSlab() can be called for the head page.
> 
> 
> It's not clear to me whether it's safer to use the...user-provided page, or the
> head page, for checking PageSlab()--it's probably set on both head and non-head
> pages, right?

PageSlab() will call compound_head() inside.

> I guess you're saying it should be like this:
> 
> 	mapcount = PageSlab(head) ? 0 : page_mapcount(page);
> 
> ...yes?

Yes.

-- 
 Kirill A. Shutemov

Re: [PATCH] mm: Improve dump_page() for compound pages

[John Hubbard]

On 2/10/20 1:54 PM, Kirill A. Shutemov wrote:
> On Mon, Feb 10, 2020 at 01:33:22PM -0800, John Hubbard wrote:
>>> PageSlab() can be called for the head page.
>>
>>
>> It's not clear to me whether it's safer to use the...user-provided page, or the
>> head page, for checking PageSlab()--it's probably set on both head and non-head
>> pages, right?
> 
> PageSlab() will call compound_head() inside.
> 
>> I guess you're saying it should be like this:
>>
>> 	mapcount = PageSlab(head) ? 0 : page_mapcount(page);
>>
>> ...yes?
> 
> Yes.
> 

OK very good, thanks for walking me through it. :)

Matthew, if you like I'll just fix it up like that for my v6 posting of the
patchset, since it's such a small change.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] mm: Improve dump_page() for compound pages

[Matthew Wilcox]

On Mon, Feb 10, 2020 at 02:00:39PM -0800, John Hubbard wrote:
> On 2/10/20 1:54 PM, Kirill A. Shutemov wrote:
> > On Mon, Feb 10, 2020 at 01:33:22PM -0800, John Hubbard wrote:
> >>> PageSlab() can be called for the head page.
> >>
> >>
> >> It's not clear to me whether it's safer to use the...user-provided page, or the
> >> head page, for checking PageSlab()--it's probably set on both head and non-head
> >> pages, right?
> > 
> > PageSlab() will call compound_head() inside.
> > 
> >> I guess you're saying it should be like this:
> >>
> >> 	mapcount = PageSlab(head) ? 0 : page_mapcount(page);
> >>
> >> ...yes?
> > 
> > Yes.
> > 
> 
> OK very good, thanks for walking me through it. :)
> 
> Matthew, if you like I'll just fix it up like that for my v6 posting of the
> patchset, since it's such a small change.

Yes, please just fix it up.