[PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Yunhui Cui]

madvise_inject_error() advances through the requested range using the
size of the page returned by get_user_pages_fast(). Saving the size
before error injection is required for hugetlb pages because successful
soft offlining can dissolve the source huge page.

That stride is incorrect for non-hugetlb large folios in system memory.
The memory failure handlers split such a folio and handle only the base
page for the supplied PFN. Advancing by the pre-split folio size then
skips the remaining pages in the requested range while madvise() still
reports success.

Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
folios are handled as a whole.

Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")
Cc: stable@vger.kernel.org
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 mm/madvise.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 5a09cc24f04a0..e9d4c3bbc5290 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1455,20 +1455,25 @@ static int madvise_inject_error(struct madvise_behavior *madv_behavior)
 
 	for (; start < end; start += size) {
 		unsigned long pfn;
+		struct folio *folio;
 		struct page *page;
 		int ret;
 
 		ret = get_user_pages_fast(start, 1, 0, &page);
 		if (ret != 1)
 			return ret;
+		folio = page_folio(page);
 		pfn = page_to_pfn(page);
 
 		/*
-		 * When soft offlining hugepages, after migrating the page
-		 * we dissolve it, therefore in the second loop "page" will
-		 * no longer be a compound page.
+		 * Non-hugetlb large folios in system memory are split and only
+		 * the addressed base page is handled. Hugetlb folios may be
+		 * dissolved and ZONE_DEVICE folios may be handled as a whole,
+		 * so save their size before error injection.
 		 */
-		size = page_size(compound_head(page));
+		size = PAGE_SIZE;
+		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
+			size = folio_size(folio);
 
 		if (madv_behavior->behavior == MADV_SOFT_OFFLINE) {
 			pr_info("Soft offlining pfn %#lx at process virtual address %#lx\n",
-- 
2.39.5

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Andrew Morton]

On Thu,  6 Aug 2026 13:55:01 +0800 Yunhui Cui <cuiyunhui@bytedance.com> wrote:

> madvise_inject_error() advances through the requested range using the
> size of the page returned by get_user_pages_fast(). Saving the size
> before error injection is required for hugetlb pages because successful
> soft offlining can dissolve the source huge page.
> 
> That stride is incorrect for non-hugetlb large folios in system memory.
> The memory failure handlers split such a folio and handle only the base
> page for the supplied PFN. Advancing by the pre-split folio size then
> skips the remaining pages in the requested range while madvise() still
> reports success.
> 
> Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
> folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
> folios are handled as a whole.

Thanks.

> Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>

Should there be an Assisted-by: here?

When fixing a bug, please always describe the userspace-visible runtime
effects of that bug.  Especially when proposing a -stable backport.


I asked Gemini this question and then pasted in your email.  It told me
stuff, but I don't know if it's correct.  In this case it would be good
to hear that description in your own words, please.

And perhaps a means of reproducing the bug - clearly our selftests/
coverage here is poor.

Re: [External] Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[yunhui cui]

Hi Andrew,

On Thu, Aug 6, 2026 at 2:29 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu,  6 Aug 2026 13:55:01 +0800 Yunhui Cui <cuiyunhui@bytedance.com> wrote:
>
> > madvise_inject_error() advances through the requested range using the
> > size of the page returned by get_user_pages_fast(). Saving the size
> > before error injection is required for hugetlb pages because successful
> > soft offlining can dissolve the source huge page.
> >
> > That stride is incorrect for non-hugetlb large folios in system memory.
> > The memory failure handlers split such a folio and handle only the base
> > page for the supplied PFN. Advancing by the pre-split folio size then
> > skips the remaining pages in the requested range while madvise() still
> > reports success.
> >
> > Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
> > folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
> > folios are handled as a whole.
>
> Thanks.
>
> > Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>
> Should there be an Assisted-by: here?
>
> When fixing a bug, please always describe the userspace-visible runtime
> effects of that bug.  Especially when proposing a -stable backport.
>
>
> I asked Gemini this question and then pasted in your email.  It told me
> stuff, but I don't know if it's correct.  In this case it would be good
> to hear that description in your own words, please.
>
> And perhaps a means of reproducing the bug - clearly our selftests/
> coverage here is poor.

The issue can be reproduced with the softoffline helper from
avocado-misc-tests. With 64K mTHP enabled:
./softoffline -m private -n 50
the test consistently failed with:
pfn matches, softoffline failed at 10

After temporarily disabling 64K mTHP:
echo never > /sys/kernel/mm/transparent_hugepage/hugepages-64kB/enabled
./softoffline -m private -n 50
the test passed in five consecutive runs:
Softoffline succeeded!

Thanks,
Yunhui

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[David Hildenbrand (Arm)]

On 8/6/26 07:55, Yunhui Cui wrote:
> madvise_inject_error() advances through the requested range using the
> size of the page returned by get_user_pages_fast(). Saving the size
> before error injection is required for hugetlb pages because successful
> soft offlining can dissolve the source huge page.
> 
> That stride is incorrect for non-hugetlb large folios in system memory.
> The memory failure handlers split such a folio and handle only the base
> page for the supplied PFN. Advancing by the pre-split folio size then
> skips the remaining pages in the requested range while madvise() still
> reports success.
> 
> Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
> folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
> folios are handled as a whole.
> 
> Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  mm/madvise.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 5a09cc24f04a0..e9d4c3bbc5290 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1455,20 +1455,25 @@ static int madvise_inject_error(struct madvise_behavior *madv_behavior)
>  
>  	for (; start < end; start += size) {
>  		unsigned long pfn;
> +		struct folio *folio;
>  		struct page *page;
>  		int ret;
>  
>  		ret = get_user_pages_fast(start, 1, 0, &page);
>  		if (ret != 1)
>  			return ret;
> +		folio = page_folio(page);
>  		pfn = page_to_pfn(page);
>  
>  		/*
> -		 * When soft offlining hugepages, after migrating the page
> -		 * we dissolve it, therefore in the second loop "page" will
> -		 * no longer be a compound page.
> +		 * Non-hugetlb large folios in system memory are split and only
> +		 * the addressed base page is handled. Hugetlb folios may be
> +		 * dissolved and ZONE_DEVICE folios may be handled as a whole,
> +		 * so save their size before error injection.
>  		 */
> -		size = page_size(compound_head(page));
> +		size = PAGE_SIZE;
> +		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> +			size = folio_size(folio);


We should never ever try deferring "how much has been mapped" from a single PTE.

While this currently works for hugetlb, it's just an anti-pattern to throw
hugetlb checks and similar around.

So this is not the way to fix it.

-- 
Cheers,

David

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Lorenzo Stoakes (ARM)]

As a newer contributor to mm please wait for feedback on a v1 from reviewers
before respinning, thanks :)

On Thu, Aug 06, 2026 at 01:55:01PM +0800, Yunhui Cui wrote:
> madvise_inject_error() advances through the requested range using the
> size of the page returned by get_user_pages_fast(). Saving the size
> before error injection is required for hugetlb pages because successful
> soft offlining can dissolve the source huge page.

But you're also changing the behaviour for MADV_HWPOISON not only
MADV_SOFT_OFFLINE? Explain?

Also mention the specific madvise flags...

>
> That stride is incorrect for non-hugetlb large folios in system memory.
> The memory failure handlers split such a folio and handle only the base
> page for the supplied PFN. Advancing by the pre-split folio size then
> skips the remaining pages in the requested range while madvise() still
> reports success.

As Andrew also asks:

How did you find out about this? Is this a theoretical issue?

Why? What? Who?...

>
> Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
> folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
> folios are handled as a whole.

You get a folio to check to see if it's DAX when you have the VMA that you can
check for DAX?...

>
> Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")

4.14...!

> Cc: stable@vger.kernel.org

Stuff that touches folio and soft offline-adjacent THP is going to be tricky to
backport correctly.

And in madvise(2):

              This feature is intended for testing of memory error-
              handling code; it is available only if the kernel was
              configured with CONFIG_MEMORY_FAILURE.

For both MADV_SOFT_OFFLINE and MADV_HWPOISON.

So I don't see why this should be backported. You might accidentally not quite
offline/poison everything you intended in a test but that isn't a bug?

Given nobody's complained in nearly a decade I don't think this matters.

Also you're suggesting backporting to pre-folio ancient times which is going to
make this horrid and tricky there too.

> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>

This could really do with a test to reproduce the issue. Mm selftests already
has memory failure stuff.

Please provide one to demonstrate the issue.

> ---
>  mm/madvise.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 5a09cc24f04a0..e9d4c3bbc5290 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1455,20 +1455,25 @@ static int madvise_inject_error(struct madvise_behavior *madv_behavior)
>
>  	for (; start < end; start += size) {
>  		unsigned long pfn;
> +		struct folio *folio;
>  		struct page *page;
>  		int ret;
>
>  		ret = get_user_pages_fast(start, 1, 0, &page);
>  		if (ret != 1)
>  			return ret;
> +		folio = page_folio(page);
>  		pfn = page_to_pfn(page);
>
>  		/*
> -		 * When soft offlining hugepages, after migrating the page
> -		 * we dissolve it, therefore in the second loop "page" will
> -		 * no longer be a compound page.
> +		 * Non-hugetlb large folios in system memory are split and only

'non-hugetlb large folios' is horrid. Large folios. And 'system memory' is
redundant.

> +		 * the addressed base page is handled. Hugetlb folios may be
> +		 * dissolved and ZONE_DEVICE folios may be handled as a whole,
> +		 * so save their size before error injection.
>  		 */

You're discarding the whole thing of the page no longer being the head. But I
guess with folios we don't need to mention that.

This whole thing can made more succinct like:

		/* DAX and hugetlb are consumed in folio chunks, everything else is split. */

> -		size = page_size(compound_head(page));

Yuck at this existing code.

> +		size = PAGE_SIZE;
> +		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> +			size = folio_size(folio);

It's really disgusting that hugetlb is treated differently with everything else
treated page-at-a-time.

Honestly I'd prefer to see a folio helper so the main loop can just assume
folio size stride.

But you can enter midway through a bloody folio. Ugh. So maybe that doesn't
work.

Let's at least split out the folio check into a helper to make things
clearer:

static bool poison_splits_folio(const struct folio *folio)
{
	/* Hugetlb is, as always, a world unto itself. */
	if (folio_test_hugetlb(folio))
		return false;
	/* Soft-offline errors out, hwpoison traverse DAX intact. */
	if (folio_is_zone_device(folio))
		return false;
	return true;
}

Then for your patch:

-		size = PAGE_SIZE;
-		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
-			size = folio_size(folio);
+		size = poison_splits_folio(folio) ? PAGE_SIZE : folio_size(folio);

I tried writing something that was neater and nicer but AI kept pointing
out how it was totally broken and I really really hate this code (not your
fault :).

--
Cheers, Lorenzo

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 10:41:06AM +0200, David Hildenbrand (Arm) wrote:
> On 8/6/26 07:55, Yunhui Cui wrote:
> > madvise_inject_error() advances through the requested range using the
> > size of the page returned by get_user_pages_fast(). Saving the size
> > before error injection is required for hugetlb pages because successful
> > soft offlining can dissolve the source huge page.
> >
> > That stride is incorrect for non-hugetlb large folios in system memory.
> > The memory failure handlers split such a folio and handle only the base
> > page for the supplied PFN. Advancing by the pre-split folio size then
> > skips the remaining pages in the requested range while madvise() still
> > reports success.
> >
> > Advance by PAGE_SIZE for non-hugetlb folios in system memory. Retain
> > folio_size() for hugetlb and ZONE_DEVICE folios, as compound Device DAX
> > folios are handled as a whole.
> >
> > Fixes: 19bfbe22f59a ("mm, hugetlb, soft_offline: save compound page order before page migration")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  mm/madvise.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 5a09cc24f04a0..e9d4c3bbc5290 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -1455,20 +1455,25 @@ static int madvise_inject_error(struct madvise_behavior *madv_behavior)
> >
> >  	for (; start < end; start += size) {
> >  		unsigned long pfn;
> > +		struct folio *folio;
> >  		struct page *page;
> >  		int ret;
> >
> >  		ret = get_user_pages_fast(start, 1, 0, &page);
> >  		if (ret != 1)
> >  			return ret;
> > +		folio = page_folio(page);
> >  		pfn = page_to_pfn(page);
> >
> >  		/*
> > -		 * When soft offlining hugepages, after migrating the page
> > -		 * we dissolve it, therefore in the second loop "page" will
> > -		 * no longer be a compound page.
> > +		 * Non-hugetlb large folios in system memory are split and only
> > +		 * the addressed base page is handled. Hugetlb folios may be
> > +		 * dissolved and ZONE_DEVICE folios may be handled as a whole,
> > +		 * so save their size before error injection.
> >  		 */
> > -		size = page_size(compound_head(page));
> > +		size = PAGE_SIZE;
> > +		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> > +			size = folio_size(folio);
>
>
> We should never ever try deferring "how much has been mapped" from a single PTE.

Inferring? :)

>
> While this currently works for hugetlb, it's just an anti-pattern to throw
> hugetlb checks and similar around.
>
> So this is not the way to fix it.

All of this code is disgusting. Also what about an address range that is
partially inside a folio...? Then presumably the whole folio is discarded? Or
does it figure it out somehow and does the split for the rest of the range?

And what if end < the end of the hugetlb size?

Ugh god I hate all of this, it's so so bad. And hugetlb being the special
snowflake is the cherry on the s*** cake...

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[David Hildenbrand (Arm)]

> 
> Let's at least split out the folio check into a helper to make things
> clearer:
> 
> static bool poison_splits_folio(const struct folio *folio)
> {
> 	/* Hugetlb is, as always, a world unto itself. */
> 	if (folio_test_hugetlb(folio))
> 		return false;
> 	/* Soft-offline errors out, hwpoison traverse DAX intact. */
> 	if (folio_is_zone_device(folio))
> 		return false;
> 	return true;
> }
> 
> Then for your patch:
> 
> -		size = PAGE_SIZE;
> -		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> -			size = folio_size(folio);
> +		size = poison_splits_folio(folio) ? PAGE_SIZE : folio_size(folio);
> 
> I tried writing something that was neater and nicer but AI kept pointing
> out how it was totally broken and I really really hate this code (not your
> fault :).

No, I don't think any such special casing on folios is the right way to handle it.

-- 
Cheers,

David

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 01:35:30PM +0200, David Hildenbrand (Arm) wrote:
> >
> > Let's at least split out the folio check into a helper to make things
> > clearer:
> >
> > static bool poison_splits_folio(const struct folio *folio)
> > {
> > 	/* Hugetlb is, as always, a world unto itself. */
> > 	if (folio_test_hugetlb(folio))
> > 		return false;
> > 	/* Soft-offline errors out, hwpoison traverse DAX intact. */
> > 	if (folio_is_zone_device(folio))
> > 		return false;
> > 	return true;
> > }
> >
> > Then for your patch:
> >
> > -		size = PAGE_SIZE;
> > -		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> > -			size = folio_size(folio);
> > +		size = poison_splits_folio(folio) ? PAGE_SIZE : folio_size(folio);
> >
> > I tried writing something that was neater and nicer but AI kept pointing
> > out how it was totally broken and I really really hate this code (not your
> > fault :).
>
> No, I don't think any such special casing on folios is the right way to handle it.

I mean the issue here is the stride varies depending on whether the thing is
hugetlb or not (and some weird DAX thing), and the poisoning causes a split
otherwise so if you want to poison a range you have to account for that.

But I agree there's something wrong here.

At first I thought 'handle at a folio granularity and assume folio slide' but
stupidly this code allows you to poison partial large folio ranges (I don't
think it should and given it's debug/testing crap I think it's fine to change
that).

But then maybe the answer is to change the interface in general? But at the same
time poisoning happens at a pfn level...

It's kinda weird to allow such an open-ended debug thing. I'd prefer we just
disallowed hugetlb, or did a split right away if THP or something.

Anyway hwpoison is a swamp of neglect and hackery regardless.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[David Hildenbrand (Arm)]

On 8/6/26 16:34, Lorenzo Stoakes (ARM) wrote:
> On Thu, Aug 06, 2026 at 01:35:30PM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> Let's at least split out the folio check into a helper to make things
>>> clearer:
>>>
>>> static bool poison_splits_folio(const struct folio *folio)
>>> {
>>> 	/* Hugetlb is, as always, a world unto itself. */
>>> 	if (folio_test_hugetlb(folio))
>>> 		return false;
>>> 	/* Soft-offline errors out, hwpoison traverse DAX intact. */
>>> 	if (folio_is_zone_device(folio))
>>> 		return false;
>>> 	return true;
>>> }
>>>
>>> Then for your patch:
>>>
>>> -		size = PAGE_SIZE;
>>> -		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
>>> -			size = folio_size(folio);
>>> +		size = poison_splits_folio(folio) ? PAGE_SIZE : folio_size(folio);
>>>
>>> I tried writing something that was neater and nicer but AI kept pointing
>>> out how it was totally broken and I really really hate this code (not your
>>> fault :).
>>
>> No, I don't think any such special casing on folios is the right way to handle it.
> 
> I mean the issue here is the stride varies depending on whether the thing is
> hugetlb or not (and some weird DAX thing), and the poisoning causes a split
> otherwise so if you want to poison a range you have to account for that.

We GUP'ed a single page and now try to be smart about which other pages we'd GUP
next.

That's just wrong, and hugetlb special-casing is just ugly.

The problem here is that, if we GUP'ed a page and poisoned it, the GUP'ing the
next page might fail and we'd return an error.

But maybe that error can simply be handled? We have FOLL_HWPOISON.

So maybe we can just use FOLL_HWPOISON and skip over the entries that already
return -EHWPOISON?

-- 
Cheers,

David

Re: [PATCH v2] mm/madvise: avoid skipping pages after splitting large folios

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 04:46:10PM +0200, David Hildenbrand (Arm) wrote:
> On 8/6/26 16:34, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Aug 06, 2026 at 01:35:30PM +0200, David Hildenbrand (Arm) wrote:
> >>>
> >>> Let's at least split out the folio check into a helper to make things
> >>> clearer:
> >>>
> >>> static bool poison_splits_folio(const struct folio *folio)
> >>> {
> >>> 	/* Hugetlb is, as always, a world unto itself. */
> >>> 	if (folio_test_hugetlb(folio))
> >>> 		return false;
> >>> 	/* Soft-offline errors out, hwpoison traverse DAX intact. */
> >>> 	if (folio_is_zone_device(folio))
> >>> 		return false;
> >>> 	return true;
> >>> }
> >>>
> >>> Then for your patch:
> >>>
> >>> -		size = PAGE_SIZE;
> >>> -		if (folio_test_hugetlb(folio) || folio_is_zone_device(folio))
> >>> -			size = folio_size(folio);
> >>> +		size = poison_splits_folio(folio) ? PAGE_SIZE : folio_size(folio);
> >>>
> >>> I tried writing something that was neater and nicer but AI kept pointing
> >>> out how it was totally broken and I really really hate this code (not your
> >>> fault :).
> >>
> >> No, I don't think any such special casing on folios is the right way to handle it.
> >
> > I mean the issue here is the stride varies depending on whether the thing is
> > hugetlb or not (and some weird DAX thing), and the poisoning causes a split
> > otherwise so if you want to poison a range you have to account for that.
>
> We GUP'ed a single page and now try to be smart about which other pages we'd GUP
> next.
>
> That's just wrong, and hugetlb special-casing is just ugly.
>
> The problem here is that, if we GUP'ed a page and poisoned it, the GUP'ing the
> next page might fail and we'd return an error.
>
> But maybe that error can simply be handled? We have FOLL_HWPOISON.
>
> So maybe we can just use FOLL_HWPOISON and skip over the entries that already
> return -EHWPOISON?

Yup this is ugly debug code so that works for me.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo