linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] mm/debug: __dump_page() prints an extra line
@ 2019-11-12  1:26 Ralph Campbell
  2019-11-12 19:55 ` Jerome Glisse
  0 siblings, 1 reply; 3+ messages in thread
From: Ralph Campbell @ 2019-11-12  1:26 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-mm, linux-kernel, Ralph Campbell

When dumping struct page information, __dump_page() prints the page type
with a trailing blank followed by the page flags on a separate line:

anon
flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)

It looks like the intent was to use pr_cont() for printing "flags:"
but pr_cont() usage is discouraged so fix this by extending the format
to include the flags into a single line:

anon flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)

If the page is file backed, the name might be long so use two lines:

shmem_aops name:"dev/zero"
flags: 0x10000000008000c(uptodate|dirty|swapbacked)

Eliminate pr_conf() usage as well for appending compound_mapcount.

Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
---
 mm/debug.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/mm/debug.c b/mm/debug.c
index 8345bb6e4769..772d4cf0691f 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -67,28 +67,31 @@ void __dump_page(struct page *page, const char *reason)
 	 */
 	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
 
-	pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx",
-		  page, page_ref_count(page), mapcount,
-		  page->mapping, page_to_pgoff(page));
 	if (PageCompound(page))
-		pr_cont(" compound_mapcount: %d", compound_mapcount(page));
-	pr_cont("\n");
+		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));
+	else
+		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
+			page, page_ref_count(page), mapcount,
+			page->mapping, page_to_pgoff(page));
 	if (PageAnon(page))
-		pr_warn("anon ");
+		pr_warn("anon flags: %#lx(%pGp)\n", page->flags, &page->flags);
 	else if (PageKsm(page))
-		pr_warn("ksm ");
+		pr_warn("ksm flags: %#lx(%pGp)\n", page->flags, &page->flags);
 	else if (mapping) {
-		pr_warn("%ps ", mapping->a_ops);
 		if (mapping->host && mapping->host->i_dentry.first) {
 			struct dentry *dentry;
 			dentry = container_of(mapping->host->i_dentry.first, struct dentry, d_u.d_alias);
-			pr_warn("name:\"%pd\" ", dentry);
-		}
+			pr_warn("%ps name:\"%pd\"\n", mapping->a_ops, dentry);
+		} else
+			pr_warn("%ps\n", mapping->a_ops);
+		pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
 	}
 	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
 
-	pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
-
 hex_only:
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
-- 
2.20.1


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

* Re: [PATCH v3] mm/debug: __dump_page() prints an extra line
  2019-11-12  1:26 [PATCH v3] mm/debug: __dump_page() prints an extra line Ralph Campbell
@ 2019-11-12 19:55 ` Jerome Glisse
  2019-11-12 21:35   ` Ralph Campbell
  0 siblings, 1 reply; 3+ messages in thread
From: Jerome Glisse @ 2019-11-12 19:55 UTC (permalink / raw)
  To: Ralph Campbell; +Cc: Andrew Morton, linux-mm, linux-kernel

On Mon, Nov 11, 2019 at 05:26:08PM -0800, Ralph Campbell wrote:
> When dumping struct page information, __dump_page() prints the page type
> with a trailing blank followed by the page flags on a separate line:
> 
> anon
> flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)
> 
> It looks like the intent was to use pr_cont() for printing "flags:"
> but pr_cont() usage is discouraged so fix this by extending the format
> to include the flags into a single line:
> 
> anon flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)
> 
> If the page is file backed, the name might be long so use two lines:
> 
> shmem_aops name:"dev/zero"
> flags: 0x10000000008000c(uptodate|dirty|swapbacked)
> 
> Eliminate pr_conf() usage as well for appending compound_mapcount.
> 
> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>

Would be nice to have a changed since v1 v2 ... i was reading my
inbox in order and i saw Andrew reply after seeing the v2 ... so
where we at here ?

> ---
>  mm/debug.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/debug.c b/mm/debug.c
> index 8345bb6e4769..772d4cf0691f 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -67,28 +67,31 @@ void __dump_page(struct page *page, const char *reason)
>  	 */
>  	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
>  
> -	pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx",
> -		  page, page_ref_count(page), mapcount,
> -		  page->mapping, page_to_pgoff(page));
>  	if (PageCompound(page))
> -		pr_cont(" compound_mapcount: %d", compound_mapcount(page));
> -	pr_cont("\n");
> +		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));
> +	else
> +		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
> +			page, page_ref_count(page), mapcount,
> +			page->mapping, page_to_pgoff(page));
>  	if (PageAnon(page))
> -		pr_warn("anon ");
> +		pr_warn("anon flags: %#lx(%pGp)\n", page->flags, &page->flags);
>  	else if (PageKsm(page))
> -		pr_warn("ksm ");
> +		pr_warn("ksm flags: %#lx(%pGp)\n", page->flags, &page->flags);
>  	else if (mapping) {
> -		pr_warn("%ps ", mapping->a_ops);
>  		if (mapping->host && mapping->host->i_dentry.first) {
>  			struct dentry *dentry;
>  			dentry = container_of(mapping->host->i_dentry.first, struct dentry, d_u.d_alias);
> -			pr_warn("name:\"%pd\" ", dentry);
> -		}
> +			pr_warn("%ps name:\"%pd\"\n", mapping->a_ops, dentry);
> +		} else
> +			pr_warn("%ps\n", mapping->a_ops);
> +		pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
>  	}
>  	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
>  
> -	pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
> -
>  hex_only:
>  	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>  			sizeof(unsigned long), page,
> -- 
> 2.20.1
> 
> 


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

* Re: [PATCH v3] mm/debug: __dump_page() prints an extra line
  2019-11-12 19:55 ` Jerome Glisse
@ 2019-11-12 21:35   ` Ralph Campbell
  0 siblings, 0 replies; 3+ messages in thread
From: Ralph Campbell @ 2019-11-12 21:35 UTC (permalink / raw)
  To: Jerome Glisse; +Cc: Andrew Morton, linux-mm, linux-kernel


On 11/12/19 11:55 AM, Jerome Glisse wrote:
> On Mon, Nov 11, 2019 at 05:26:08PM -0800, Ralph Campbell wrote:
>> When dumping struct page information, __dump_page() prints the page type
>> with a trailing blank followed by the page flags on a separate line:
>>
>> anon
>> flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)
>>
>> It looks like the intent was to use pr_cont() for printing "flags:"
>> but pr_cont() usage is discouraged so fix this by extending the format
>> to include the flags into a single line:
>>
>> anon flags: 0x100000000090034(uptodate|lru|active|head|swapbacked)
>>
>> If the page is file backed, the name might be long so use two lines:
>>
>> shmem_aops name:"dev/zero"
>> flags: 0x10000000008000c(uptodate|dirty|swapbacked)
>>
>> Eliminate pr_conf() usage as well for appending compound_mapcount.
>>
>> Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
> 
> Would be nice to have a changed since v1 v2 ... i was reading my
> inbox in order and i saw Andrew reply after seeing the v2 ... so
> where we at here ?

V3 is the latest and looks like Andrew has queued it for mm.
My bad, I should have had the version changes listed in the cover
letter portion of the email.
Andrew commented on v1, v2 simply fixed the subject line which
was truncated in v1, and v3 is as you see.

Actually, I now see another bug which I'll post separately:
The "if (PageAnon(page))" will always trigger so that
"else if (PageKsm(page))" will never be executed.

>> ---
>>   mm/debug.c | 27 +++++++++++++++------------
>>   1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/mm/debug.c b/mm/debug.c
>> index 8345bb6e4769..772d4cf0691f 100644
>> --- a/mm/debug.c
>> +++ b/mm/debug.c
>> @@ -67,28 +67,31 @@ void __dump_page(struct page *page, const char *reason)
>>   	 */
>>   	mapcount = PageSlab(page) ? 0 : page_mapcount(page);
>>   
>> -	pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx",
>> -		  page, page_ref_count(page), mapcount,
>> -		  page->mapping, page_to_pgoff(page));
>>   	if (PageCompound(page))
>> -		pr_cont(" compound_mapcount: %d", compound_mapcount(page));
>> -	pr_cont("\n");
>> +		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));
>> +	else
>> +		pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
>> +			page, page_ref_count(page), mapcount,
>> +			page->mapping, page_to_pgoff(page));
>>   	if (PageAnon(page))
>> -		pr_warn("anon ");
>> +		pr_warn("anon flags: %#lx(%pGp)\n", page->flags, &page->flags);
>>   	else if (PageKsm(page))
>> -		pr_warn("ksm ");
>> +		pr_warn("ksm flags: %#lx(%pGp)\n", page->flags, &page->flags);
>>   	else if (mapping) {
>> -		pr_warn("%ps ", mapping->a_ops);
>>   		if (mapping->host && mapping->host->i_dentry.first) {
>>   			struct dentry *dentry;
>>   			dentry = container_of(mapping->host->i_dentry.first, struct dentry, d_u.d_alias);
>> -			pr_warn("name:\"%pd\" ", dentry);
>> -		}
>> +			pr_warn("%ps name:\"%pd\"\n", mapping->a_ops, dentry);
>> +		} else
>> +			pr_warn("%ps\n", mapping->a_ops);
>> +		pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
>>   	}
>>   	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
>>   
>> -	pr_warn("flags: %#lx(%pGp)\n", page->flags, &page->flags);
>> -
>>   hex_only:
>>   	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>>   			sizeof(unsigned long), page,
>> -- 
>> 2.20.1
>>
>>
> 

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

end of thread, other threads:[~2019-11-12 21:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12  1:26 [PATCH v3] mm/debug: __dump_page() prints an extra line Ralph Campbell
2019-11-12 19:55 ` Jerome Glisse
2019-11-12 21:35   ` Ralph Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).