| 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: <314b6f14-7740-487f-8b39-0ed1e54b4782@oracle.com>
Date: Tue, 20 Jan 2026 15:23:10 -0800
From: jane.chu@...cle.com
To: Miaohe Lin <linmiaohe@...wei.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 1/20/2026 3:54 AM, Miaohe Lin wrote:
> 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.
Thanks a lot! v7 sent out.
-jane
> .
Powered by blists - more mailing lists