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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39F5AC433F5 for ; Fri, 5 Nov 2021 18:24:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A637D61131 for ; Fri, 5 Nov 2021 18:24:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A637D61131 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id E47A46B0071; Fri, 5 Nov 2021 14:24:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DF7726B0072; Fri, 5 Nov 2021 14:24:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C97CD940007; Fri, 5 Nov 2021 14:24:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0215.hostedemail.com [216.40.44.215]) by kanga.kvack.org (Postfix) with ESMTP id BA0656B0071 for ; Fri, 5 Nov 2021 14:24:00 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 6789876BB7 for ; Fri, 5 Nov 2021 18:24:00 +0000 (UTC) X-FDA: 78775700640.30.2473E92 Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf31.hostedemail.com (Postfix) with ESMTP id CFD8F104AACE for ; Fri, 5 Nov 2021 18:23:51 +0000 (UTC) Received: by mail-ed1-f43.google.com with SMTP id x15so4778194edv.1 for ; Fri, 05 Nov 2021 11:23:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GRxn0JMjSx331eMdc46Cl3J9iqi8wXcpf71hHNMClSI=; b=gMxncrJYIzEWUSmC2Vxihm3mtIHSybTCQTcgD+HqvbAtbTsVPtW0+bvXhnEUtabJ+L 2CHb8jQnyTsqiR3mcowujAi79jG1cfuc6f6GbjI/3ZyoT6+OUIkS2JPixJHXk4Qm6yRR +9BiyWyaD1fNojh4kYJoqvLJB7ryQ9GdoZyfIfp83Fy/5MzUkNHloLCLu1HhGCNFhEf4 CKxJrjAUn7PkpMcQOwX6G240E5e4ZR/LVzimqdPz3NvA31ygIQgcw6Hu5rNOj6tNjqm0 6mtlZSXHFnVzxHEgg+g6vRVq4zwkKeJu227E7vY9GK9TWDs7GCuY364TGseb8gTCpwpC E9dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GRxn0JMjSx331eMdc46Cl3J9iqi8wXcpf71hHNMClSI=; b=CS04DZAcAUzJQ0lypklS9omakqVcZWTy7SREHpziu26a3lG7lSACuAYBkZzxl3v+L+ rXOksUe06h/cYQJzDT4v3CpFdV5ntcKkY265HIdSSNd8evhEPBKFlYS5v2k7uvXXMi5j HvHOkC1xvr8riYUHxovGoB7kWuh15kRZhrVNtXbz5van8R6lkx5f0z1tYDg6RhWjhBFy r0J1PZ8K/rTbb/JnjuQbxKeMPg5CDpGLH7Jko9vCz04KA0CsljI4ABNqYuap3TG/ul+c b4A4jYphHvU52hcujR42TVJYSsR1JswvwbG3aji1qhYlSVqzFeyv09Fme0oiR3bYMKGx mf9g== X-Gm-Message-State: AOAM531qM1OfhVYjLebU3/Q8sqCpL9NSh/VNQKcxouHCaZZryStlX1zf IYqwrJQ60g/WrSPiEwy8XHA3vwZ+J30ReAHqcvs= X-Google-Smtp-Source: ABdhPJxXw1zMHW+/GOi3DtAZN9HvvWSw78MAshex0BHFNL+eq32WosimuxDuk9SKmOVyAWHvk3T9j+S0/wn5yeDJlWg= X-Received: by 2002:a17:906:a94b:: with SMTP id hh11mr75486317ejb.85.1636136638763; Fri, 05 Nov 2021 11:23:58 -0700 (PDT) MIME-Version: 1.0 References: <20211105055058.3152564-1-naoya.horiguchi@linux.dev> <20211105055058.3152564-2-naoya.horiguchi@linux.dev> In-Reply-To: <20211105055058.3152564-2-naoya.horiguchi@linux.dev> From: Yang Shi Date: Fri, 5 Nov 2021 11:23:47 -0700 Message-ID: Subject: Re: [PATCH v3 1/3] mm/hwpoison: mf_mutex for soft offline and unpoison To: Naoya Horiguchi Cc: Linux MM , Andrew Morton , David Hildenbrand , Oscar Salvador , Michal Hocko , Ding Hui , Tony Luck , "Aneesh Kumar K.V" , Miaohe Lin , Peter Xu , Naoya Horiguchi , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=gMxncrJY; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf31.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: CFD8F104AACE X-Stat-Signature: 4q7m5kdm1zwubrjj9x3tdskb81tsdkx5 X-HE-Tag: 1636136631-415531 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, Nov 4, 2021 at 10:52 PM Naoya Horiguchi wrote: > > From: Naoya Horiguchi > > Originally mf_mutex is introduced to serialize multiple MCE events, but > it is not that useful to allow unpoison to run in parallel with memory_failure() > and soft offline. So apply mf_they to soft offline and unpoison. ^^^^^^^^ A typo? It should be mf_mutex, right? Looks good to me other than the above nit. Reviewed-by: Yang Shi > The memory failure handler and soft offline handler get simpler with this. > > Signed-off-by: Naoya Horiguchi > --- > ChangeLog v3: > - merge with "mm/hwpoison: remove race consideration" > - update description > > ChangeLog v2: > - add mutex_unlock() in "page already poisoned" path in soft_offline_page(). > (Thanks to Ding Hui) > --- > mm/memory-failure.c | 62 +++++++++++++-------------------------------- > 1 file changed, 18 insertions(+), 44 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index e8c38e27b753..d29c79de6034 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -1507,14 +1507,6 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) > lock_page(head); > page_flags = head->flags; > > - if (!PageHWPoison(head)) { > - pr_err("Memory failure: %#lx: just unpoisoned\n", pfn); > - num_poisoned_pages_dec(); > - unlock_page(head); > - put_page(head); > - return 0; > - } > - > /* > * TODO: hwpoison for pud-sized hugetlb doesn't work right now, so > * simply disable it. In order to make it work properly, we need > @@ -1628,6 +1620,8 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, > return rc; > } > > +static DEFINE_MUTEX(mf_mutex); > + > /** > * memory_failure - Handle memory failure of a page. > * @pfn: Page Number of the corrupted page > @@ -1654,7 +1648,6 @@ int memory_failure(unsigned long pfn, int flags) > int res = 0; > unsigned long page_flags; > bool retry = true; > - static DEFINE_MUTEX(mf_mutex); > > if (!sysctl_memory_failure_recovery) > panic("Memory failure on page %lx", pfn); > @@ -1788,16 +1781,6 @@ int memory_failure(unsigned long pfn, int flags) > */ > page_flags = p->flags; > > - /* > - * unpoison always clear PG_hwpoison inside page lock > - */ > - if (!PageHWPoison(p)) { > - pr_err("Memory failure: %#lx: just unpoisoned\n", pfn); > - num_poisoned_pages_dec(); > - unlock_page(p); > - put_page(p); > - goto unlock_mutex; > - } > if (hwpoison_filter(p)) { > if (TestClearPageHWPoison(p)) > num_poisoned_pages_dec(); > @@ -1978,6 +1961,7 @@ int unpoison_memory(unsigned long pfn) > struct page *page; > struct page *p; > int freeit = 0; > + int ret = 0; > unsigned long flags = 0; > static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL, > DEFAULT_RATELIMIT_BURST); > @@ -1988,39 +1972,30 @@ int unpoison_memory(unsigned long pfn) > p = pfn_to_page(pfn); > page = compound_head(p); > > + mutex_lock(&mf_mutex); > + > if (!PageHWPoison(p)) { > unpoison_pr_info("Unpoison: Page was already unpoisoned %#lx\n", > pfn, &unpoison_rs); > - return 0; > + goto unlock_mutex; > } > > if (page_count(page) > 1) { > unpoison_pr_info("Unpoison: Someone grabs the hwpoison page %#lx\n", > pfn, &unpoison_rs); > - return 0; > + goto unlock_mutex; > } > > if (page_mapped(page)) { > unpoison_pr_info("Unpoison: Someone maps the hwpoison page %#lx\n", > pfn, &unpoison_rs); > - return 0; > + goto unlock_mutex; > } > > if (page_mapping(page)) { > unpoison_pr_info("Unpoison: the hwpoison page has non-NULL mapping %#lx\n", > pfn, &unpoison_rs); > - return 0; > - } > - > - /* > - * unpoison_memory() can encounter thp only when the thp is being > - * worked by memory_failure() and the page lock is not held yet. > - * In such case, we yield to memory_failure() and make unpoison fail. > - */ > - if (!PageHuge(page) && PageTransHuge(page)) { > - unpoison_pr_info("Unpoison: Memory failure is now running on %#lx\n", > - pfn, &unpoison_rs); > - return 0; > + goto unlock_mutex; > } > > if (!get_hwpoison_page(p, flags)) { > @@ -2028,29 +2003,23 @@ int unpoison_memory(unsigned long pfn) > num_poisoned_pages_dec(); > unpoison_pr_info("Unpoison: Software-unpoisoned free page %#lx\n", > pfn, &unpoison_rs); > - return 0; > + goto unlock_mutex; > } > > - lock_page(page); > - /* > - * This test is racy because PG_hwpoison is set outside of page lock. > - * That's acceptable because that won't trigger kernel panic. Instead, > - * the PG_hwpoison page will be caught and isolated on the entrance to > - * the free buddy page pool. > - */ > if (TestClearPageHWPoison(page)) { > unpoison_pr_info("Unpoison: Software-unpoisoned page %#lx\n", > pfn, &unpoison_rs); > num_poisoned_pages_dec(); > freeit = 1; > } > - unlock_page(page); > > put_page(page); > if (freeit && !(pfn == my_zero_pfn(0) && page_count(p) == 1)) > put_page(page); > > - return 0; > +unlock_mutex: > + mutex_unlock(&mf_mutex); > + return ret; > } > EXPORT_SYMBOL(unpoison_memory); > > @@ -2231,9 +2200,12 @@ int soft_offline_page(unsigned long pfn, int flags) > return -EIO; > } > > + mutex_lock(&mf_mutex); > + > if (PageHWPoison(page)) { > pr_info("%s: %#lx page already poisoned\n", __func__, pfn); > put_ref_page(ref_page); > + mutex_unlock(&mf_mutex); > return 0; > } > > @@ -2251,5 +2223,7 @@ int soft_offline_page(unsigned long pfn, int flags) > } > } > > + mutex_unlock(&mf_mutex); > + > return ret; > } > -- > 2.25.1 > >