Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2] mm, dump_page: do not crash with bad
 compound_mapcount()
To:     "Kirill A. Shutemov" <kirill@shutemov.name>,
        Matthew Wilcox <willy@infradead.org>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, <linux-mm@kvack.org>,
        <cai@lca.pw>, <rppt@linux.ibm.com>, <vbabka@suse.cz>,
        <william.kucharski@oracle.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
References: <20200804214807.169256-1-jhubbard@nvidia.com>
 <20200807143504.4kudtd4xeoqaroqg@box>
 <20200807151029.GE17456@casper.infradead.org>
 <20200807164805.xm4ingj4crdiemol@box>
From:   John Hubbard <jhubbard@nvidia.com>
Message-ID: <2038d28f-0f65-5629-f033-e92c5c5e9798@nvidia.com>
Date:   Fri, 7 Aug 2020 15:40:21 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <20200807164805.xm4ingj4crdiemol@box>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 8/7/20 9:48 AM, Kirill A. Shutemov wrote:
>> [...]
>>>> +static inline int head_mapcount(struct page *head)
>>>> +{
>>>
>>> Do we want VM_BUG_ON_PAGE(!PageHead(head), head) here?
>>
>> Well, no.  That was the point of the bug report -- by the time we called
>> compound_mapcount, the page was no longer a head page.
> 
> Right. VM_BUG_ON_PAGE(PageTail(head), head)?

Sorry for overlooking that feedback point. Looking at it now, I'd much
rather not put any assertions at all here. This supposed to be for
implementing the failure case, and we've moved past assertions at this
point. In other words, dump_page() is part of *implementing* an
assertion, so calling assertions from within it is undesirable.

It's better to put the assertions in code that would call these inner
routines. Then, if we want to use these routines outside of mm/debug.c,
as per the other thread, then we should provide a wrapper that has such
assertions.


thanks,
-- 
John Hubbard
NVIDIA