| 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
| ||
|
Message-ID: <958f1e3a-3c40-51ae-8fac-a185e76aa940@huawei.com>
Date: Tue, 20 Jan 2026 19:54:12 +0800
From: Miaohe Lin <linmiaohe@...wei.com>
To: Jane Chu <jane.chu@...cle.com>
CC: <linux-mm@...ck.org>, <stable@...r.kernel.org>, <muchun.song@...ux.dev>,
<osalvador@...e.de>, <david@...nel.org>, <jiaqiyan@...gle.com>,
<william.roche@...cle.com>, <rientjes@...gle.com>,
<akpm@...ux-foundation.org>, <lorenzo.stoakes@...cle.com>,
<Liam.Howlett@...cle.com>, <rppt@...nel.org>, <surenb@...gle.com>,
<mhocko@...e.com>, <willy@...radead.org>, <clm@...a.com>, linux-kernel
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 1/2] mm/memory-failure: fix missing ->mf_stats count in
hugetlb poison
On 2026/1/17 4:38, Jane Chu wrote:
> When a newly poisoned subpage ends up in an already poisoned hugetlb
> folio, 'num_poisoned_pages' is incremented, but the per node ->mf_stats
> is not. Fix the inconsistency by designating action_result() to update
> them both.
>
> While at it, define __get_huge_page_for_hwpoison() return values in terms
> of symbol names for better readibility. Also rename
> folio_set_hugetlb_hwpoison() to hugetlb_update_hwpoison() since the
> function does more than the conventional bit setting and the fact
> three possible return values are expected.
>
> Fixes: 18f41fa616ee ("mm: memory-failure: bump memory failure stats to pglist_data")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Jane Chu <jane.chu@...cle.com>
This patch looks good to me with some nits below.
Acked-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
> v5 -> v6:
> comments from Miaohe.
> v5 -> v4:
> fix a bug pointed out by William and Chris, add comment.
> v3 -> v4:
> incorporate/adapt David's suggestions.
> v2 -> v3:
> No change.
> v1 -> v2:
> adapted David and Liam's comment, define __get_huge_page_for_hwpoison()
> return values in terms of symbol names instead of naked integers for better
> readibility. #define instead of enum is used since the function has footprint
> outside MF, just try to limit the MF specifics local.
> also renamed folio_set_hugetlb_hwpoison() to hugetlb_update_hwpoison()
> since the function does more than the conventional bit setting and the
> fact three possible return values are expected.
>
> ---
> mm/memory-failure.c | 91 +++++++++++++++++++++++++++------------------
> 1 file changed, 54 insertions(+), 37 deletions(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index c80c2907da33..49ced16e9c1a 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1883,12 +1883,22 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
> return count;
> }
>
> -static int folio_set_hugetlb_hwpoison(struct folio *folio, struct page *page)
> +#define MF_HUGETLB_FREED 0 /* freed hugepage */
> +#define MF_HUGETLB_IN_USED 1 /* in-use hugepage */
> +#define MF_HUGETLB_NON_HUGEPAGE 2 /* not a hugepage */
> +#define MF_HUGETLB_FOLIO_PRE_POISONED 3 /* folio already poisoned */
> +#define MF_HUGETLB_PAGE_PRE_POISONED 4 /* exact page already poisoned */
> +#define MF_HUGETLB_RETRY 5 /* hugepage is busy, retry */
> +/*
> + * Set hugetlb folio as hwpoisoned, update folio private raw hwpoison list
> + * to keep track of the poisoned pages.
> + */
> +static int hugetlb_update_hwpoison(struct folio *folio, struct page *page)
> {
> struct llist_head *head;
> struct raw_hwp_page *raw_hwp;
> struct raw_hwp_page *p;
> - int ret = folio_test_set_hwpoison(folio) ? -EHWPOISON : 0;
> + int ret = folio_test_set_hwpoison(folio) ? MF_HUGETLB_FOLIO_PRE_POISONED : 0;
>
> /*
> * Once the hwpoison hugepage has lost reliable raw error info,
> @@ -1896,20 +1906,17 @@ static int folio_set_hugetlb_hwpoison(struct folio *folio, struct page *page)
> * so skip to add additional raw error info.
> */
> if (folio_test_hugetlb_raw_hwp_unreliable(folio))
> - return -EHWPOISON;
> + return MF_HUGETLB_FOLIO_PRE_POISONED;
> head = raw_hwp_list_head(folio);
> llist_for_each_entry(p, head->first, node) {
> if (p->page == page)
> - return -EHWPOISON;
> + return MF_HUGETLB_PAGE_PRE_POISONED;
> }
>
> raw_hwp = kmalloc(sizeof(struct raw_hwp_page), GFP_ATOMIC);
> if (raw_hwp) {
> raw_hwp->page = page;
> llist_add(&raw_hwp->node, head);
> - /* the first error event will be counted in action_result(). */
> - if (ret)
> - num_poisoned_pages_inc(page_to_pfn(page));
> } else {
> /*
> * Failed to save raw error info. We no longer trace all
> @@ -1957,42 +1964,38 @@ void folio_clear_hugetlb_hwpoison(struct folio *folio)
>
> /*
> * Called from hugetlb code with hugetlb_lock held.
> - *
> - * Return values:
> - * 0 - free hugepage
> - * 1 - in-use hugepage
> - * 2 - not a hugepage
> - * -EBUSY - the hugepage is busy (try to retry)
> - * -EHWPOISON - the hugepage is already hwpoisoned
> */
> int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
> bool *migratable_cleared)
> {
> struct page *page = pfn_to_page(pfn);
> struct folio *folio = page_folio(page);
> - int ret = 2; /* fallback to normal page handling */
> bool count_increased = false;
> + int ret, rc;
>
> - if (!folio_test_hugetlb(folio))
> + if (!folio_test_hugetlb(folio)) {
> + ret = MF_HUGETLB_NON_HUGEPAGE;
> goto out;
> -
> - if (flags & MF_COUNT_INCREASED) {
> - ret = 1;
> + } else if (flags & MF_COUNT_INCREASED) {
> + ret = MF_HUGETLB_IN_USED;
> count_increased = true;
> } else if (folio_test_hugetlb_freed(folio)) {
> - ret = 0;
> + ret = MF_HUGETLB_FREED;
> } else if (folio_test_hugetlb_migratable(folio)) {
> - ret = folio_try_get(folio);
> - if (ret)
> + if (folio_try_get(folio)) {
> + ret = MF_HUGETLB_IN_USED;
> count_increased = true;
> + } else
> + ret = MF_HUGETLB_FREED;
IIRC, code style requires {} here. .i.e
if (folio_try_get(folio)) {
ret = MF_HUGETLB_IN_USED;
count_increased = true;
} else {
ret = MF_HUGETLB_FREED;
}
> } else {
> - ret = -EBUSY;
> + ret = MF_HUGETLB_RETRY;
> if (!(flags & MF_NO_RETRY))
> goto out;
> }
>
> - if (folio_set_hugetlb_hwpoison(folio, page)) {
> - ret = -EHWPOISON;
> + rc = hugetlb_update_hwpoison(folio, page);
> + if (rc >= MF_HUGETLB_FOLIO_PRE_POISONED) {
> + ret = rc;
> goto out;
> }
>
> @@ -2017,10 +2020,15 @@ int __get_huge_page_for_hwpoison(unsigned long pfn, int flags,
> * with basic operations like hugepage allocation/free/demotion.
> * So some of prechecks for hwpoison (pinning, and testing/setting
> * PageHWPoison) should be done in single hugetlb_lock range.
> + * Returns:
> + * 0 - not hugetlb, or recovered
> + * -EBUSY - not recovered
> + * -EOPNOTSUPP - hwpoison_filter'ed
> + * -EHWPOISON - folio or exact page already poisoned
-EFAULT can be returned when kill_accessing_process finds p->mm is null. So it might be better
to comment EFAULT case too.
Thanks.
.
Powered by blists - more mailing lists