From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 149DCC433DB for ; Thu, 18 Mar 2021 14:58:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 4BA8064F11 for ; Thu, 18 Mar 2021 14:58:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4BA8064F11 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8B3456B0072; Thu, 18 Mar 2021 10:58:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 863456B0073; Thu, 18 Mar 2021 10:58:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 703D26B0074; Thu, 18 Mar 2021 10:58:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0140.hostedemail.com [216.40.44.140]) by kanga.kvack.org (Postfix) with ESMTP id 532706B0072 for ; Thu, 18 Mar 2021 10:58:32 -0400 (EDT) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 130007597 for ; Thu, 18 Mar 2021 14:58:32 +0000 (UTC) X-FDA: 77933301180.08.965D66F Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf01.hostedemail.com (Postfix) with ESMTP id 8727C5000099 for ; Thu, 18 Mar 2021 14:57:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=OPpj51tbQS/2//vcHFyB7cf2IjN9iGh+azR89eU09G4=; b=TJvatpsdhA3/Ue4L0m1KEToBq/ MQeGHoBBFr76wvtFoqK/HtWUfQpEb34sXqKn2q3q5aI3XAR6SEtmGbFPhERBRWCRgi+30qj0326LW 6Dndbp2rPS2BBYy+3G82JzMu+9C9fjIpoZocyY3eN2oj12Vd/G6+1sAXFEt7iYQ1s2TGtlRKaMXKz gtnCRJM2K1jQaYU3Ei3O7QgngpGuC8Kj1eTS1Isamgt619Lg82Vez0VFJV3kX2gY7GHNyedCLMg0d 3xIqMqT6ORF/UNv1cfC+NWHw/NcAst1YSAG621wr4WtKQQmcovbUl5Hyg37vYz96/O82xjUAz/3HU 8UbiSBnw==; Received: from willy by casper.infradead.org with local (Exim 4.94 #2 (Red Hat Linux)) id 1lMu5M-0036eG-8t; Thu, 18 Mar 2021 14:57:19 +0000 Date: Thu, 18 Mar 2021 14:57:16 +0000 From: Matthew Wilcox To: "Kirill A. Shutemov" Cc: Naoya Horiguchi , linux-mm@kvack.org, "Kirill A. Shutemov" , Hugh Dickins , Andi Kleen Subject: Re: File THP and HWPoison Message-ID: <20210318145716.GO3420@casper.infradead.org> References: <20210316140947.GA3420@casper.infradead.org> <20210318140843.7dv3wnxg4geplrjx@box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210318140843.7dv3wnxg4geplrjx@box> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 8727C5000099 X-Stat-Signature: hzx6hf97gzzk1r7w3u4nnrbfddx4tdwk Received-SPF: none (infradead.org>: No applicable sender policy available) receiver=imf01; identity=mailfrom; envelope-from=""; helo=casper.infradead.org; client-ip=90.155.50.34 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616079478-773918 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Mar 18, 2021 at 05:08:43PM +0300, Kirill A. Shutemov wrote: > On Tue, Mar 16, 2021 at 02:09:47PM +0000, Matthew Wilcox wrote: > > If we get a memory failure in the middle of a file THP, I think we handle > > it poorly. > > > > int memory_failure(unsigned long pfn, int flags) > > ... > > if (TestSetPageHWPoison(p)) { > > ... > > orig_head = hpage = compound_head(p); > > ... > > if (PageTransHuge(hpage)) { > > if (try_to_split_thp_page(p, "Memory Failure") < 0) { > > action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED); > > return -EBUSY; > > } > > > > static int try_to_split_thp_page(struct page *page, const char *msg) > > { > > lock_page(page); > > if (!PageAnon(page) || unlikely(split_huge_page(page))) { > > unsigned long pfn = page_to_pfn(page); > > > > unlock_page(page); > > if (!PageAnon(page)) > > pr_info("%s: %#lx: non anonymous thp\n", msg, pfn); > > else > > pr_info("%s: %#lx: thp split failed\n", msg, pfn); > > put_page(page); > > return -EBUSY; > > > > So (for some reason) we don't even try to split a file THP. But then, > > if we take a page fault on a file THP: > > > > static struct page *next_uptodate_page(struct page *page, > > ... > > if (PageHWPoison(page)) > > goto skip; > > (... but we're only testing the head page here, which isn't necessarily > > the one which got the error ...) > > > > if (pmd_none(*vmf->pmd) && PageTransHuge(page)) { > > vm_fault_t ret = do_set_pmd(vmf, page); > > > > So we now map the PMD-sized page into userspace, even though it has a > > HWPoison in it. > > > > I think there are two things that we should be doing: > > > > 1. Attempt to split THPs which are file-backed. That makes most of this > > problem disappear because there won't be THPs with HWPoison, mostly. > > +Naoya. Could you give more context here? I did some git archaeology and found this check was introduced in 7f6bf39bbdd1 ("mm/hwpoison: fix panic due to split huge zero page") where it wasn't intended to catch _file_ pages at all, but the zero page. I suspect that nobody thought to look at this when introducing THP for shmem. > > 2. When the THP fails to split, use a spare page flag to indicate that > > the THP contains a HWPoison bit in one of its subpages. There are a > > lot of PF_SECOND flags available for this purpose. > > > > but I know almost nothing about the memory-failure subsystem and I'm > > still learning all the complexities of THPs, so it's entirely possible > > I've overlooked something important. > > I wounder if it would be cleaner to switch PG_hwpoison to PF_HEAD: if > split failed we posion whole compound page. Yes, we will waste more > memory, but it makes it much cleaner for user: just check if the page is > poisoned. I think that's a poor quality implementation ... it'd cause processes to die that weren't even touching the page that had hwpoison. Using a PF_SECOND bit lets us do the check as cheaply as if we made hwpoison PF_HEAD.