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=-24.8 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1,USER_IN_DEF_DKIM_WL autolearn=ham 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 C2C7AC433DB for ; Thu, 11 Mar 2021 07:22:40 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6DCB064FAA for ; Thu, 11 Mar 2021 07:22:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6DCB064FAA Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DF4C18D027A; Thu, 11 Mar 2021 02:22:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DCB238D0250; Thu, 11 Mar 2021 02:22:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C6C7E8D027A; Thu, 11 Mar 2021 02:22:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0194.hostedemail.com [216.40.44.194]) by kanga.kvack.org (Postfix) with ESMTP id AD7D88D0250 for ; Thu, 11 Mar 2021 02:22:38 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 76758180ACC31 for ; Thu, 11 Mar 2021 07:22:38 +0000 (UTC) X-FDA: 77906750796.12.FFD7226 Received: from mail-oi1-f176.google.com (mail-oi1-f176.google.com [209.85.167.176]) by imf04.hostedemail.com (Postfix) with ESMTP id 1EC3A3C1 for ; Thu, 11 Mar 2021 07:22:35 +0000 (UTC) Received: by mail-oi1-f176.google.com with SMTP id d16so12849636oic.0 for ; Wed, 10 Mar 2021 23:22:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=RsQ39ePrjlJasxWu8MVkxDuRqN5EB8evI6SJn9j1EnU=; b=AIjzzh9q2zyaY726QVOjA0aj0rGQPJH+obw7Et0eMFaYLT9ts1idc51z/32DpIkAkp wct/7+bN30NLOf6F0le39pV/ZfXAqH4YF2dwflp7qM6Sy0G+MEM0lW+D6F032XxRrdPT /iTQl3oOjVtXDm4vo/BejdUg8nWfPMs2u+Nnu+P+P76JKxrukV6y8jFRHgJzHxzLPkZu A0oO0uD2WoGXE/IF5RtQBRCTCG6701xx2T77SAlX2YPeFT7oAArK13LvdfDzhBI8Hp69 HarO3zmmtdDwqPWjBnw/ErP3MtPYZon2Y+4nLPPYvYGtlRApiD04xoJgg2VgkFY/KKVk /Nfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=RsQ39ePrjlJasxWu8MVkxDuRqN5EB8evI6SJn9j1EnU=; b=pgezKMlrn8zAuSTSmLG/OoADkwiMRcPA70zNb+4mya6UaJ2BiPH7TolR9/KOTfYDxg s63IFATjqhe37N+z706iJ18tf+DET3KXMaJED39IRE+CK6t9+qx6Zadv2zQq9HvGeIHG XwbsvMdMVjlfqCoye1b6W2R31YUEJB6Y/lxV7mosYSSo7N/itJLCzi0dlDmWrV6uk50T UOfEKn8WQ/+6ivnsNnT5DclENkStxf2/GDRPGMrr2vMuI9UoypQrM0+7EEUcoPKSYa+f soAxKEv3rCqNNElmtkf9cqmUq3GUOSJ2sUAQDBZ0vpxzNIs/V0NvDBzx2uFnXeQQhV3V m62g== X-Gm-Message-State: AOAM5309lCg/mnXM85S8wuMYT0tu7FOVgSp6HA1xyi6/q3UwwMVxGX+R HAOYqWOMBFkVWgbm7RkxY/TRUQ== X-Google-Smtp-Source: ABdhPJzdIpyPXOpylsYda1l/POaA65Jwuh+iDDw6Ir/wtgpFbG80Sue/PDNHHrIkwAG/gpyV7Dh+rg== X-Received: by 2002:aca:7543:: with SMTP id q64mr5416901oic.100.1615447355057; Wed, 10 Mar 2021 23:22:35 -0800 (PST) Received: from eggly.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id w5sm424095oig.24.2021.03.10.23.22.33 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Wed, 10 Mar 2021 23:22:34 -0800 (PST) Date: Wed, 10 Mar 2021 23:22:18 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Naoya Horiguchi cc: Andrew Morton , Michal Hocko , Oscar Salvador , Tony Luck , Matthew Wilcox , "Aneesh Kumar K.V" , Naoya Horiguchi , Jue Wang , Greg Thelen , Hugh Dickins , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v1] mm, hwpoison: enable error handling on shmem thp In-Reply-To: <20210209062128.453814-1-nao.horiguchi@gmail.com> Message-ID: References: <20210209062128.453814-1-nao.horiguchi@gmail.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1EC3A3C1 X-Stat-Signature: sorgkb9s9y8wndh4ni6ssetuwc55gas8 Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf04; identity=mailfrom; envelope-from=""; helo=mail-oi1-f176.google.com; client-ip=209.85.167.176 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1615447355-456523 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 Tue, 9 Feb 2021, Naoya Horiguchi wrote: > From: Naoya Horiguchi > > Currently hwpoison code checks PageAnon() for thp and refuses to handle > errors on non-anonymous thps (just for historical reason). We now > support non-anonymou thp like shmem one, so this patch suggests to enable > to handle shmem thps. Fortunately, we already have can_split_huge_page() > to check if a give thp is splittable, so this patch relies on it. Fortunately? I don't understand. Why call can_split_huge_page() at all, instead of simply trying split_huge_page() directly? And could it do better than -EBUSY when split_huge_page() fails? > > Signed-off-by: Naoya Horiguchi Thanks for trying to add shmem+file THP support, but I think this does not work as intended - Andrew, if Naoya agrees, please drop from mmotm for now, the fixup needed will be more than a line or two. I'm not much into memory-failure myself, but Jue discovered that the SIGBUS never arrives: because split_huge_page() on a shmem or file THP unmaps all its pmds and ptes, and (unlike with anon) leaves them unmapped - in normal circumstances, to be faulted back on demand. So the page_mapped() check in hwpoison_user_mappings() fails, and the intended SIGBUS is not delivered. (Or, is it acceptable that the SIGBUS is not delivered to those who have the huge page mapped: should it get delivered later, to anyone who faults back in the bad 4k?) We believe the tokill list has to be set up earlier, before split_huge_page() is called, then passed in to hwpoison_user_mappings(). Sorry, we don't have a proper patch for that right now, but I expect you can see what needs to be done. But something we found on the way, we do have a patch for: add_to_kill() uses page_address_in_vma(), but that has not been used on file THP tails before - fix appended at the end below, so as not to waste your time on that bit. > --- > mm/memory-failure.c | 34 +++++++++------------------------- > 1 file changed, 9 insertions(+), 25 deletions(-) > > diff --git v5.11-rc6/mm/memory-failure.c v5.11-rc6_patched/mm/memory-failure.c > index e9481632fcd1..8c1575de0507 100644 > --- v5.11-rc6/mm/memory-failure.c > +++ v5.11-rc6_patched/mm/memory-failure.c > @@ -956,20 +956,6 @@ static int __get_hwpoison_page(struct page *page) > { > struct page *head = compound_head(page); > > - if (!PageHuge(head) && PageTransHuge(head)) { > - /* > - * Non anonymous thp exists only in allocation/free time. We > - * can't handle such a case correctly, so let's give it up. > - * This should be better than triggering BUG_ON when kernel > - * tries to touch the "partially handled" page. > - */ > - if (!PageAnon(head)) { > - pr_err("Memory failure: %#lx: non anonymous thp\n", > - page_to_pfn(page)); > - return 0; > - } > - } > - > if (get_page_unless_zero(head)) { > if (head == compound_head(page)) > return 1; > @@ -1197,21 +1183,19 @@ static int identify_page_state(unsigned long pfn, struct page *p, > > 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); > + struct page *head; > > + lock_page(page); > + head = compound_head(page); > + if (PageTransHuge(head) && can_split_huge_page(head, NULL) && > + !split_huge_page(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; > + return 0; > } > + pr_info("%s: %#lx: thp split failed\n", msg, page_to_pfn(page)); > unlock_page(page); > - > - return 0; > + put_page(page); > + return -EBUSY; > } > > static int memory_failure_hugetlb(unsigned long pfn, int flags) > -- > 2.25.1 [PATCH] mm: fix page_address_in_vma() on file THP tails From: Jue Wang Anon THP tails were already supported, but memory-failure now needs to use page_address_in_vma() on file THP tails, which its page->mapping check did not permit: fix it. Signed-off-by: Jue Wang Signed-off-by: Hugh Dickins --- mm/rmap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) --- 5.12-rc2/mm/rmap.c 2021-02-28 16:58:57.950450151 -0800 +++ linux/mm/rmap.c 2021-03-10 20:29:21.591475177 -0800 @@ -717,11 +717,11 @@ unsigned long page_address_in_vma(struct if (!vma->anon_vma || !page__anon_vma || vma->anon_vma->root != page__anon_vma->root) return -EFAULT; - } else if (page->mapping) { - if (!vma->vm_file || vma->vm_file->f_mapping != page->mapping) - return -EFAULT; - } else + } else if (!vma->vm_file) { + return -EFAULT; + } else if (vma->vm_file->f_mapping != compound_head(page)->mapping) { return -EFAULT; + } address = __vma_address(page, vma); if (unlikely(address < vma->vm_start || address >= vma->vm_end)) return -EFAULT;