linux-kernel - Re: [PATCH v2] mm/hwpoison: use pr_err() instead of dump_page() in get_any

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: <20220510103158.GA174652@hori.linux.bs1.fc.nec.co.jp>
Date:   Tue, 10 May 2022 10:31:58 +0000
From:   HORIGUCHI NAOYA(堀口　直也) 
        <naoya.horiguchi@....com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     Naoya Horiguchi <naoya.horiguchi@...ux.dev>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Christoph Hellwig <hch@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        John Hubbard <jhubbard@...dia.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        William Kucharski <william.kucharski@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] mm/hwpoison: use pr_err() instead of dump_page() in
 get_any_page()

On Tue, May 10, 2022 at 03:28:05AM +0100, Matthew Wilcox wrote:
> On Wed, Apr 27, 2022 at 02:32:20PM +0900, Naoya Horiguchi wrote:
> > From: Naoya Horiguchi <naoya.horiguchi@....com>
> > 
> > The following VM_BUG_ON_FOLIO() is triggered when memory error event
> > happens on the (thp/folio) pages which are about to be freed:
> 
> So the real problem is that we're calling dump_page() when we don't
> have a reference to the page, right?  Otherwise it wouldn't be freed.

Right, that's the problem.

> 
> >  out:
> >  	if (ret == -EIO)
> > -		dump_page(p, "hwpoison: unhandlable page");
> > +		pr_err("Memory failure: %#lx: unhandlable page.\n", page_to_pfn(p));
> 
> It would be nice to get some more information out of the page than that
> ,.. but taking a refcount inside dump_page() conflicts with the other
> "would be nice", which is for dump_page() to take a const struct page *
> so we can (eg) make folio_test_uptodate() take a const struct folio *.
> 
> We've had some other problems with inconsistent pages being printed in
> dump_page().  It can be quite confusing when debugging.  I still don't
> have a good solution to that either.
> 
> I do have a proposal for reforming mapcount which will solve this
> particular problem, but I'm not quite sure when I'll get to it.
> This patch is probably the best thing to do for now.

Thank you for the comment.  So I leave this code simple (or some
hwpoison specific dump_page() if someone have good ideas) until some
ultimate solution is ready for upstream.

Thanks,
Naoya Horiguchi