Re: [PATCH] mm/memory-failure: fix refcount leak on soft-offline LBS folio
Andrew Morton <[email protected]> Mon, 3 Aug 2026 15:06:46 -0700
| Newsgroups | org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 3 Aug 2026 14:00:01 +0800 [email protected] wrote: > From: liyouhong <[email protected]> > > soft_offline_in_use_page() runs with a folio reference taken by > get_hwpoison_page() (or by the caller when MF_COUNT_INCREASED is set, > e.g. madvise(MADV_SOFT_OFFLINE)). > > made soft-offline skip splitting when min_order_for_split() Something went wrong with this paragraph? > is non-zero, so large-block-size folios stay intact. That early > return -EBUSY path never drops the reference. So the effect is to leak a folio ref if min_order_for_split() returns 0. Has this been known to happen in the real world? It would be good to update soft_offline_in_use_page()'s introductory comment to describe its folio refcount handling. > The other failure path is fine: try_to_split_thp_page(..., release=true) > puts the page when the split itself fails. memory_failure() also puts > explicitly on its analogous unsplit path. > > Split the new_order != 0 case out and folio_put() before returning. > > ... > > --- a/mm/memory-failure.c > +++ b/mm/memory-failure.c > @@ -2801,9 +2801,19 @@ static int soft_offline_in_use_page(struct page *page) > * NOTE: if minimizing the number of soft offline pages is > * preferred, split it to non-zero new_order like it is done in > * memory_failure(). > + * > + * Drop the ref from get_hwpoison_page()/MF_COUNT_INCREASED; > + * try_to_split_thp_page(..., release=true) does that itself > + * when the split fails. > */ > - if (new_order || try_to_split_thp_page(page, /* new_order= */ 0, > - /* release= */ true)) { > + if (new_order) { > + pr_info("%#lx: order-%d folio cannot soft offline\n", > + pfn, new_order); > + folio_put(folio); > + return -EBUSY; > + } > + if (try_to_split_thp_page(page, /* new_order= */ 0, > + /* release= */ true)) { > pr_info("%#lx: thp split failed\n", pfn); > return -EBUSY; > } Thanks, lgtm. AI review might have found a couple of pre-existing issues in the surrounding code. From a quick read, it doesn't seem appropriate to address either within the context of this change. https://sashiko.dev/#/patchset/[email protected]