From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout06.his.huawei.com (canpmsgout06.his.huawei.com [113.46.200.221]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 04A4826CE2D for ; Thu, 15 Jan 2026 03:41:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.221 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768448516; cv=none; b=s7Ie7mXt40XDXyCZZ9vZLGLN3LuOSMg0+ippsV3rk7ksG64yJVYeKO7J3O8p3E0lAIakgf+9+75ys+muMN7akwf6b3ool4S2CefHtOAdh2d98sfsOqYWhmqP3Ks6bsjSFkOFBs03aabDI5v5VFfoNA1Ev5kw6N2t7Qqb5aWlByo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768448516; c=relaxed/simple; bh=bBXdE4thZQFxeQHR+Ya98YGwkXvN+7bn6ABoLaOBUUo=; h=Subject:To:CC:References:From:Message-ID:Date:MIME-Version: In-Reply-To:Content-Type; b=HYp8SnKwtWqer2c79V7uPijb75He5qxKSwGAGdIhpjali9nk8S4Miw9z96qR/PCjDn/gLXwkwN5BunpdwruZ4f5Mw/ZqBmAvAC8BfBO/cVd5ju2WaKnARok8sxKaFeGA900tz+z5W/bHekQs0WziApziv7sJ0NnYrOoIW+mKxqk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=imeh2gQy; arc=none smtp.client-ip=113.46.200.221 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="imeh2gQy" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=MVQCjDWM4p7/HO/QYULMb9geDprLDOiFu5R6foDcOj0=; b=imeh2gQyFkyUWsl5mFaJuNUGqN7XbF0qR8ILODVsp4Lb0q1mS5ISKDudG6BOgs/xEeqIGSDqb t42QeVxLA1OtUYhGGmRj29WPtY3i3bJ2Cr1zNNwlp6rrKZJeuZTsckkgqpb3fl3ooAGGSLl0kjy MXUdXoxcs/IAeMJhDvOzKFM= Received: from mail.maildlp.com (unknown [172.19.162.140]) by canpmsgout06.his.huawei.com (SkyGuard) with ESMTPS id 4ds7xL30bfzRhRw; Thu, 15 Jan 2026 11:38:30 +0800 (CST) Received: from dggemv705-chm.china.huawei.com (unknown [10.3.19.32]) by mail.maildlp.com (Postfix) with ESMTPS id B9AB0201E9; Thu, 15 Jan 2026 11:41:50 +0800 (CST) Received: from kwepemq500010.china.huawei.com (7.202.194.235) by dggemv705-chm.china.huawei.com (10.3.19.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Thu, 15 Jan 2026 11:41:50 +0800 Received: from [10.173.125.37] (10.173.125.37) by kwepemq500010.china.huawei.com (7.202.194.235) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Thu, 15 Jan 2026 11:41:49 +0800 Subject: Re: [PATCH v3 3/3] mm/memory-failure: refactor page_handle_poison() To: Jiaqi Yan CC: , , , , , , , , , , , , , , , , , , , , , , , , References: <20260112004923.888429-1-jiaqiyan@google.com> <20260112004923.888429-4-jiaqiyan@google.com> From: Miaohe Lin Message-ID: Date: Thu, 15 Jan 2026 11:41:48 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <20260112004923.888429-4-jiaqiyan@google.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-ClientProxiedBy: kwepems100002.china.huawei.com (7.221.188.206) To kwepemq500010.china.huawei.com (7.202.194.235) On 2026/1/12 8:49, Jiaqi Yan wrote: > Now that HWPoison page(s) within HugeTLB page will be rejected by > buddy allocator during dissolve_free_hugetlb_folio(), there is no > need to drain_all_pages() and take_page_off_buddy() anymore. In fact, > calling take_page_off_buddy() after dissolve_free_hugetlb_folio() > succeeded returns false, making caller think page_handl_poion() failed. s/page_handl_poion/page_handle_poison/ > > On the other hand, for hardware corrupted pages in buddy allocator, > take_page_off_buddy() is still a must-have. > > Given hugepage and free buddy page should be treated differently, > refactor page_handle_poison() and __page_handle_poison(): > > - __page_handle_poison() is unwind into page_handle_poison(). > > - Callers of page_handle_poison() also need to explicitly tell if > page is HugeTLB hugepage or free buddy page. > > - Add helper hugepage_handle_poison() for several existing HugeTLB > specific callsites. > > Signed-off-by: Jiaqi Yan > --- > mm/memory-failure.c | 84 ++++++++++++++++++++++----------------------- > 1 file changed, 41 insertions(+), 43 deletions(-) > > diff --git a/mm/memory-failure.c b/mm/memory-failure.c > index d204de6c9792a..1fdaee1e48bb8 100644 > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -162,54 +162,48 @@ static struct rb_root_cached pfn_space_itree = RB_ROOT_CACHED; > > static DEFINE_MUTEX(pfn_space_lock); > > -/* > - * Return values: > - * 1: the page is dissolved (if needed) and taken off from buddy, > - * 0: the page is dissolved (if needed) and not taken off from buddy, > - * < 0: failed to dissolve. > +/** > + * Handle the HugeTLB hugepage that @page belongs to. Return values: > + * = 0: the hugepage is free hugepage and is dissolved. In soft offline scene, dissolve_free_hugetlb_folio would return 0 when the page becomes a normal page due to race. > + * < 0: hugepage is in-use or failed to dissolve. > */ > -static int __page_handle_poison(struct page *page) > +static int hugepage_handle_poison(struct page *page) > { > - int ret; > + return dissolve_free_hugetlb_folio(page_folio(page)); > +} > + > +/** > + * Helper at the end of handling @page having hardware errors. > + * @huge: @page is part of a HugeTLB hugepage. > + * @free: @page is free buddy page. > + * @release: memory-failure module should release a pending refcount. > + */ > +static bool page_handle_poison(struct page *page, bool huge, bool free, > + bool release) > +{ > + int ret = 0; > > /* > - * zone_pcp_disable() can't be used here. It will > - * hold pcp_batch_high_lock and dissolve_free_hugetlb_folio() might hold > - * cpu_hotplug_lock via static_key_slow_dec() when hugetlb vmemmap > - * optimization is enabled. This will break current lock dependency > - * chain and leads to deadlock. > - * Disabling pcp before dissolving the page was a deterministic > - * approach because we made sure that those pages cannot end up in any > - * PCP list. Draining PCP lists expels those pages to the buddy system, > - * but nothing guarantees that those pages do not get back to a PCP > - * queue if we need to refill those. > + * Buddy allocator will exclude the HWPoison page after hugepage > + * is successfully dissolved. > */ > - ret = dissolve_free_hugetlb_folio(page_folio(page)); > - if (!ret) { > + if (huge) > + ret = hugepage_handle_poison(page); > + > + if (free) { Nit: huge and free won't be both true. So we could write it as: if (huge) { ... } else if (free) { > drain_all_pages(page_zone(page)); > - ret = take_page_off_buddy(page); > + ret = take_page_off_buddy(page) ? 0 : -1; > } > > - return ret; > -} > - > -static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, bool release) > -{ > - if (hugepage_or_freepage) { > + if ((huge || free) && ret < 0) Nit: ret won't be <0 if both huge and free are false. So I think we might simplify it as: if (ret < 0) > /* > - * Doing this check for free pages is also fine since > - * dissolve_free_hugetlb_folio() returns 0 for non-hugetlb folios as well. > + * We could fail to take off the target page from buddy > + * for example due to racy page allocation, but that's > + * acceptable because soft-offlined page is not broken > + * and if someone really want to use it, they should > + * take it. > */ > - if (__page_handle_poison(page) <= 0) > - /* > - * We could fail to take off the target page from buddy > - * for example due to racy page allocation, but that's > - * acceptable because soft-offlined page is not broken > - * and if someone really want to use it, they should > - * take it. > - */ > - return false; > - } > + return false; > > SetPageHWPoison(page); > if (release) > @@ -1174,7 +1168,7 @@ static int me_huge_page(struct page_state *ps, struct page *p) > * subpages. > */ > folio_put(folio); > - if (__page_handle_poison(p) > 0) { > + if (!hugepage_handle_poison(p)) { > page_ref_inc(p); > res = MF_RECOVERED; > } else { > @@ -2067,7 +2061,7 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb > */ > if (res == 0) { > folio_unlock(folio); > - if (__page_handle_poison(p) > 0) { > + if (!hugepage_handle_poison(p)) { > page_ref_inc(p); > res = MF_RECOVERED; > } else { > @@ -2815,7 +2809,7 @@ static int soft_offline_in_use_page(struct page *page) > > if (ret) { > pr_info("%#lx: invalidated\n", pfn); > - page_handle_poison(page, false, true); > + page_handle_poison(page, false, false, true); > return 0; > } > > @@ -2836,7 +2830,7 @@ static int soft_offline_in_use_page(struct page *page) > if (!ret) { > bool release = !huge; > > - if (!page_handle_poison(page, huge, release)) > + if (!page_handle_poison(page, huge, false, release)) This might not work for soft offline. PageHWPoison is not yet set so folio_clear_hugetlb_hwpoison won't be called when dissolve hugetlb hugepages... > ret = -EBUSY; > } else { > if (!list_empty(&pagelist)) > @@ -2884,6 +2878,8 @@ int soft_offline_page(unsigned long pfn, int flags) > { > int ret; > bool try_again = true; > + bool huge; > + bool free; > struct page *page; > > if (!pfn_valid(pfn)) { > @@ -2929,7 +2925,9 @@ int soft_offline_page(unsigned long pfn, int flags) > if (ret > 0) { > ret = soft_offline_in_use_page(page); > } else if (ret == 0) { > - if (!page_handle_poison(page, true, false)) { > + huge = folio_test_hugetlb(page_folio(page)); folio_test_hugetlb check is racy because there's no guarantee that hugetlb hugepage won't be dissolved before calling page_handle_poison. That will lead to problem... soft_offline_page folio_test_hugetlb -- true now page_handle_poison /* Hugepage is dissolved somewhere. */ hugepage_handle_poison -- return 0 because page is normal page or free buddy page. SetPageHWPoison(page); page_ref_inc(page); -- refcnt is increased while page might be on buddy... > + free = is_free_buddy_page(page); > + if (!page_handle_poison(page, huge, free, false)) { We assume free is always true due to ret is 0. So we can write it as: if (!page_handle_poison(page, huge, true, false)) { > if (try_again) { > try_again = false; > flags &= ~MF_COUNT_INCREASED; > Thanks. .