linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
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>
Subject: Re: [PATCH v2] mm, dump_page: do not crash with bad compound_mapcount()
Date: Fri, 7 Aug 2020 15:40:21 -0700	[thread overview]
Message-ID: <2038d28f-0f65-5629-f033-e92c5c5e9798@nvidia.com> (raw)
In-Reply-To: <20200807164805.xm4ingj4crdiemol@box>

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


      parent reply	other threads:[~2020-08-07 22:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-04 21:48 [PATCH v2] mm, dump_page: do not crash with bad compound_mapcount() John Hubbard
2020-08-06 11:45 ` Vlastimil Babka
2020-08-06 13:48   ` Matthew Wilcox
2020-08-06 15:13     ` Vlastimil Babka
2020-08-06 15:39       ` Matthew Wilcox
2020-08-06 15:53         ` Vlastimil Babka
2020-08-06 17:15           ` Matthew Wilcox
2020-08-07 14:53             ` Kirill A. Shutemov
2020-08-07 14:35 ` Kirill A. Shutemov
2020-08-07 15:10   ` Matthew Wilcox
2020-08-07 16:48     ` Kirill A. Shutemov
2020-08-07 18:33       ` [PATCH] mm, dump_page: rename head_mapcount() --> head_compound_mapcount() John Hubbard
2020-08-07 22:40       ` John Hubbard [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2038d28f-0f65-5629-f033-e92c5c5e9798@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=cai@lca.pw \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rppt@linux.ibm.com \
    --cc=vbabka@suse.cz \
    --cc=william.kucharski@oracle.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).