Re: [PATCH v3] mm/debug: __dump_page() prints an extra line

[Ralph Campbell <rcampbell@nvidia.com>]

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
>>
>>
>