linux-kernel - Re: [PATCH v6 12/12] mm: dump_page(): additional diagnostics for huge pinned pages

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <909f4ad5-128f-b4bd-e4cb-787885167a97@nvidia.com>
Date:   Tue, 11 Feb 2020 18:10:50 -0800
From:   John Hubbard <jhubbard@...dia.com>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Chinner <david@...morbit.com>,
        Ira Weiny <ira.weiny@...el.com>, Jan Kara <jack@...e.cz>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Jonathan Corbet <corbet@....net>,
        Jérôme Glisse <jglisse@...hat.com>,
        Michal Hocko <mhocko@...e.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Shuah Khan <shuah@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Matthew Wilcox <willy@...radead.org>,
        <linux-doc@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
        <linux-kselftest@...r.kernel.org>, <linux-rdma@...r.kernel.org>,
        <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v6 12/12] mm: dump_page(): additional diagnostics for huge
 pinned pages

On 2/11/20 5:21 AM, Kirill A. Shutemov wrote:
...
>> diff --git a/mm/debug.c b/mm/debug.c
>> index f5ffb0784559..2189357f0987 100644
>> --- a/mm/debug.c
>> +++ b/mm/debug.c
>> @@ -85,11 +85,22 @@ void __dump_page(struct page *page, const char *reason)
>>  	mapcount = PageSlab(head) ? 0 : page_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));
>> +		if (hpage_pincount_available(page)) {
>> +			pr_warn("page:%px refcount:%d mapcount:%d mapping:%p "
>> +				"index:%#lx head:%px order:%u "
>> +				"compound_mapcount:%d compound_pincount:%d\n",
>> +				page, page_ref_count(head), mapcount,
>> +				mapping, page_to_pgoff(page), head,
>> +				compound_order(head), compound_mapcount(page),
>> +				compound_pincount(page));
>> +		} else {
>> +			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));
>> +		}
> 
> Have you considered using pr_cont() here. I guess it would be easier to
> read.

Yes, and it does have the advantage of removing some of the code duplication above. 
On the other hand, though, it leaves the end result (the long lines being printed) 
the same, and introduces a window in which the output can get garbled by another 
thread that is printk'-ing. And actually, what I'd really like is to shorten the
printed output lines, as I mentioned in [1].

So overall, given that this series has been fairly difficult to get finalized, 
and it's now in Andrew's tree at last, I'd *really* like to leave it as-is right 
now, and build on top of it. So I will submit a follow-on patch to formally propose
shortening the printed lines, and that can live or die independently of this series,
which is hopefully over now.

> 
> You can use my Ack anyway.


Thanks, and I appreciate all of your reviews and bug spotting and ideas for improvements 
on this series, it's been really helpful.


> 
> 
>>  	else
>>  		pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n",
>>  			page, page_ref_count(page), mapcount,
>> -- 
>> 2.25.0
>>
> 


[1] https://lore.kernel.org/r/96e1f693-0e7b-2817-f13d-1946ff7654a1@nvidia.com

thanks,
-- 
John Hubbard
NVIDIA