linux-kernel - Re: [PATCH v3] mm/debug: __dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <06cc3e15-f65a-b582-1e6b-884244768351@nvidia.com>
Date:   Tue, 12 Nov 2019 13:35:35 -0800
From:   Ralph Campbell <rcampbell@...dia.com>
To:     Jerome Glisse <jglisse@...hat.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] mm/debug: __dump_page() prints an extra line


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@...dia.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
>>
>>
>