[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)]

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

[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

[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

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

[yunhui cui]

Hi Andrew, David, Lorenzo,

On Thu, Aug 6, 2026 at 11:40 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> 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.

Thank you for the review. Based on your feedback, I went back through the
madvise, GUP, soft-offline, and memory-failure paths and outlined the
changes I plan to make for the next revision.

The issue is that using a page obtained for one address to infer how far
the range walker can advance is the wrong abstraction.

For an anonymous large folio, soft_offline_page() splits the folio to
order-0 and handles only the supplied base-page PFN. Advancing by the
pre-split folio size can therefore skip the remaining base pages while
madvise() still returns success.

Lorenzo also raised the semantics of a range that covers only part of a
hugetlb page. Looking at a range that crosses a hugetlb boundary exposes
another problem. For example, with two 2 MiB hugepages:

hugepage A: [0, 2 MiB)
hugepage B: [2 MiB, 4 MiB)
requested range: [2 MiB - 4 KiB, 2 MiB + 4 KiB)

The first GUP resolves the last base page in hugepage A. Adding the full
2 MiB hugepage size to that unaligned address produces the next address
at 4 MiB - 4 KiB. That is already beyond the requested end at
2 MiB + 4 KiB, so the loop terminates without ever visiting hugepage B.

For MADV_SOFT_OFFLINE:

- ordinary pages and large folios advance by PAGE_SIZE because
  soft_offline_page() handles the supplied base-page PFN after any split;

- hugetlb advances to the end of the current hugepage because successful
  soft-offline migrates the complete hugepage and leaves a healthy
  replacement mapped. If the walker advanced by PAGE_SIZE, its next GUP
  would resolve that healthy replacement and soft-offline the same virtual
  hugepage again;

- ZONE_DEVICE does not need a stride case because soft_offline_page()
  rejects it.

Advancing to the current hugepage boundary, rather than adding the hugepage
size to the original unaligned address, lets the next iteration start
exactly at hugepage B.

For MADV_HWPOISON, I plan to follow David's suggestion and walk at
PAGE_SIZE using:

get_user_pages_unlocked(start, 1, &page,
FOLL_GET | FOLL_HWPOISON)

get_user_pages_unlocked() is the appropriate interface here because the
current gup_fast_fallback() flag mask rejects FOLL_HWPOISON, while the
memory-failure madvise path enters madvise_inject_error() without
mmap_lock held. get_user_pages_unlocked() acquires and releases mmap_lock
internally, handles fault retries, and propagates -EHWPOISON from the
fault path. FOLL_GET makes the page-reference ownership consumed by
MF_COUNT_INCREASED explicit.

A successful GUP is followed by memory_failure(). If GUP returns
-EHWPOISON, the address was already covered by an earlier larger-granularity
injection, so the walker continues with the next base-page address. Other
errors are returned. This avoids hugetlb, DAX, and folio-size inference in
the MADV_HWPOISON caller.

Device DAX is relevant only to MADV_HWPOISON because
MADV_SOFT_OFFLINE rejects ZONE_DEVICE pages. Since the proposed
MADV_HWPOISON walker advances by PAGE_SIZE and uses each GUP result as
feedback rather than inferring the handled range from folio_size(), it
should also avoid the same granularity problem for Device DAX. A
successful GUP is passed to memory_failure(), while -EHWPOISON indicates
that the address was already covered by an earlier injection. Advancing
by PAGE_SIZE should therefore also work for Device DAX in principle. I do
not currently have a suitable Device DAX setup, so this remains untested
at runtime.

Because MADV_SOFT_OFFLINE must advance past a hugetlb replacement while
MADV_HWPOISON can use FOLL_HWPOISON feedback during a PAGE_SIZE walk, I
plan to use separate walking models for the two operations.

Before posting another revision, I plan to split the work into:

1. the MADV_SOFT_OFFLINE range-walk fix;
2. MADV_SOFT_OFFLINE large-folio and hugetlb selftests;
3. the PAGE_SIZE + FOLL_HWPOISON MADV_HWPOISON walker;
4. MADV_HWPOISON large-folio and hugetlb selftests.

Does this separation of the SOFT_OFFLINE and HWPOISON walking models look
reasonable?

For stable, would you agree that I should omit the explicit stable Cc from
the next revision?

>
> >
> > --
> > Cheers,
> >
> > David
>
> --
> Cheers, Lorenzo

Thanks，
Yunhui

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

[Lorenzo Stoakes (ARM)]

On Tue, Aug 11, 2026 at 10:31:21AM +0800, yunhui cui wrote:
> Hi Andrew, David, Lorenzo,
>
> On Thu, Aug 6, 2026 at 11:40 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
> >
> > 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.
>
> Thank you for the review. Based on your feedback, I went back through the
> madvise, GUP, soft-offline, and memory-failure paths and outlined the
> changes I plan to make for the next revision.
>
> The issue is that using a page obtained for one address to infer how far
> the range walker can advance is the wrong abstraction.
>
> For an anonymous large folio, soft_offline_page() splits the folio to
> order-0 and handles only the supplied base-page PFN. Advancing by the
> pre-split folio size can therefore skip the remaining base pages while
> madvise() still returns success.
>
> Lorenzo also raised the semantics of a range that covers only part of a
> hugetlb page. Looking at a range that crosses a hugetlb boundary exposes
> another problem. For example, with two 2 MiB hugepages:
>
> hugepage A: [0, 2 MiB)
> hugepage B: [2 MiB, 4 MiB)
> requested range: [2 MiB - 4 KiB, 2 MiB + 4 KiB)
>
> The first GUP resolves the last base page in hugepage A. Adding the full
> 2 MiB hugepage size to that unaligned address produces the next address
> at 4 MiB - 4 KiB. That is already beyond the requested end at
> 2 MiB + 4 KiB, so the loop terminates without ever visiting hugepage B.
>
> For MADV_SOFT_OFFLINE:
>
> - ordinary pages and large folios advance by PAGE_SIZE because
>   soft_offline_page() handles the supplied base-page PFN after any split;
>
> - hugetlb advances to the end of the current hugepage because successful
>   soft-offline migrates the complete hugepage and leaves a healthy
>   replacement mapped. If the walker advanced by PAGE_SIZE, its next GUP
>   would resolve that healthy replacement and soft-offline the same virtual
>   hugepage again;
>
> - ZONE_DEVICE does not need a stride case because soft_offline_page()
>   rejects it.
>
> Advancing to the current hugepage boundary, rather than adding the hugepage
> size to the original unaligned address, lets the next iteration start
> exactly at hugepage B.
>
> For MADV_HWPOISON, I plan to follow David's suggestion and walk at
> PAGE_SIZE using:
>
> get_user_pages_unlocked(start, 1, &page,
> FOLL_GET | FOLL_HWPOISON)
>
> get_user_pages_unlocked() is the appropriate interface here because the
> current gup_fast_fallback() flag mask rejects FOLL_HWPOISON, while the
> memory-failure madvise path enters madvise_inject_error() without
> mmap_lock held. get_user_pages_unlocked() acquires and releases mmap_lock
> internally, handles fault retries, and propagates -EHWPOISON from the
> fault path. FOLL_GET makes the page-reference ownership consumed by
> MF_COUNT_INCREASED explicit.
>
> A successful GUP is followed by memory_failure(). If GUP returns
> -EHWPOISON, the address was already covered by an earlier larger-granularity
> injection, so the walker continues with the next base-page address. Other
> errors are returned. This avoids hugetlb, DAX, and folio-size inference in
> the MADV_HWPOISON caller.
>
> Device DAX is relevant only to MADV_HWPOISON because
> MADV_SOFT_OFFLINE rejects ZONE_DEVICE pages. Since the proposed
> MADV_HWPOISON walker advances by PAGE_SIZE and uses each GUP result as
> feedback rather than inferring the handled range from folio_size(), it
> should also avoid the same granularity problem for Device DAX. A
> successful GUP is passed to memory_failure(), while -EHWPOISON indicates
> that the address was already covered by an earlier injection. Advancing
> by PAGE_SIZE should therefore also work for Device DAX in principle. I do
> not currently have a suitable Device DAX setup, so this remains untested
> at runtime.
>
> Because MADV_SOFT_OFFLINE must advance past a hugetlb replacement while
> MADV_HWPOISON can use FOLL_HWPOISON feedback during a PAGE_SIZE walk, I
> plan to use separate walking models for the two operations.
>
> Before posting another revision, I plan to split the work into:
>
> 1. the MADV_SOFT_OFFLINE range-walk fix;
> 2. MADV_SOFT_OFFLINE large-folio and hugetlb selftests;
> 3. the PAGE_SIZE + FOLL_HWPOISON MADV_HWPOISON walker;
> 4. MADV_HWPOISON large-folio and hugetlb selftests.
>
> Does this separation of the SOFT_OFFLINE and HWPOISON walking models look
> reasonable?
>
> For stable, would you agree that I should omit the explicit stable Cc from
> the next revision?

Sigh. This reads like you've just got an LLM to write up a summary.

And prior to the slop era I _never_ saw a summary like this, ever. People would
_reply to the reviewers_ like we were human beings and not prompt-generators.

I've seen this enough times now from sloppers that I believe the intent here is
to workslop reviewers into checking the plan so that response can be fed back
into the LLM as a prompt to write the patch for them.

And I'm not sure I'm interested in seeing a v3 from you given that.

Please read:

https://docs.kernel.org/process/coding-assistants.html
https://docs.kernel.org/process/generated-content.html

And follow correct kernel process on this.

--
Cheers, Lorenzo

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

[David Hildenbrand (Arm)]

On 8/11/26 04:31, yunhui cui wrote:
> Hi Andrew, David, Lorenzo,
> 
> On Thu, Aug 6, 2026 at 11:40 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>>
>> On Thu, Aug 06, 2026 at 04:46:10PM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> 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.
> 
> Thank you for the review. Based on your feedback, I went back through the
> madvise, GUP, soft-offline, and memory-failure paths and outlined the
> changes I plan to make for the next revision.
> 
> The issue is that using a page obtained for one address to infer how far
> the range walker can advance is the wrong abstraction.
> 
> For an anonymous large folio, soft_offline_page() splits the folio to
> order-0 and handles only the supplied base-page PFN. Advancing by the
> pre-split folio size can therefore skip the remaining base pages while
> madvise() still returns success.
> 
> Lorenzo also raised the semantics of a range that covers only part of a
> hugetlb page. Looking at a range that crosses a hugetlb boundary exposes
> another problem. For example, with two 2 MiB hugepages:
> 
> hugepage A: [0, 2 MiB)
> hugepage B: [2 MiB, 4 MiB)
> requested range: [2 MiB - 4 KiB, 2 MiB + 4 KiB)
> 
> The first GUP resolves the last base page in hugepage A. Adding the full
> 2 MiB hugepage size to that unaligned address produces the next address
> at 4 MiB - 4 KiB. That is already beyond the requested end at
> 2 MiB + 4 KiB, so the loop terminates without ever visiting hugepage B.
> 
> For MADV_SOFT_OFFLINE:
> 
> - ordinary pages and large folios advance by PAGE_SIZE because
>   soft_offline_page() handles the supplied base-page PFN after any split;
> 
> - hugetlb advances to the end of the current hugepage because successful
>   soft-offline migrates the complete hugepage and leaves a healthy
>   replacement mapped. If the walker advanced by PAGE_SIZE, its next GUP
>   would resolve that healthy replacement and soft-offline the same virtual
>   hugepage again;
> 
> - ZONE_DEVICE does not need a stride case because soft_offline_page()
>   rejects it.
> 
> Advancing to the current hugepage boundary, rather than adding the hugepage
> size to the original unaligned address, lets the next iteration start
> exactly at hugepage B.
> 
> For MADV_HWPOISON, I plan to follow David's suggestion and walk at
> PAGE_SIZE using:
> 
> get_user_pages_unlocked(start, 1, &page,
> FOLL_GET | FOLL_HWPOISON)
> 
> get_user_pages_unlocked() is the appropriate interface here because the
> current gup_fast_fallback() flag mask rejects FOLL_HWPOISON, while the
> memory-failure madvise path enters madvise_inject_error() without
> mmap_lock held. get_user_pages_unlocked() acquires and releases mmap_lock
> internally, handles fault retries, and propagates -EHWPOISON from the
> fault path. FOLL_GET makes the page-reference ownership consumed by
> MF_COUNT_INCREASED explicit.
> 
> A successful GUP is followed by memory_failure(). If GUP returns
> -EHWPOISON, the address was already covered by an earlier larger-granularity
> injection, so the walker continues with the next base-page address. Other
> errors are returned. This avoids hugetlb, DAX, and folio-size inference in
> the MADV_HWPOISON caller.
> 
> Device DAX is relevant only to MADV_HWPOISON because
> MADV_SOFT_OFFLINE rejects ZONE_DEVICE pages. Since the proposed
> MADV_HWPOISON walker advances by PAGE_SIZE and uses each GUP result as
> feedback rather than inferring the handled range from folio_size(), it
> should also avoid the same granularity problem for Device DAX. A
> successful GUP is passed to memory_failure(), while -EHWPOISON indicates
> that the address was already covered by an earlier injection. Advancing
> by PAGE_SIZE should therefore also work for Device DAX in principle. I do
> not currently have a suitable Device DAX setup, so this remains untested
> at runtime.
> 
> Because MADV_SOFT_OFFLINE must advance past a hugetlb replacement while
> MADV_HWPOISON can use FOLL_HWPOISON feedback during a PAGE_SIZE walk, I
> plan to use separate walking models for the two operations.
> 
> Before posting another revision, I plan to split the work into:
> 
> 1. the MADV_SOFT_OFFLINE range-walk fix;
> 2. MADV_SOFT_OFFLINE large-folio and hugetlb selftests;
> 3. the PAGE_SIZE + FOLL_HWPOISON MADV_HWPOISON walker;
> 4. MADV_HWPOISON large-folio and hugetlb selftests.
> 
> Does this separation of the SOFT_OFFLINE and HWPOISON walking models look
> reasonable?
As Lorenzo says, this reads AI generated.

I assume what you mean is:

MADV_HWPOISON will actually hwpoison the pages. We can just use FOLL_HWPOISON +
-EHWPOISON and should be good. So far the theory.

MADV_SOFT_OFFLINE will migrate pages instead. So if we don't skip multiple
pages, we could end up migrating multiple times (and setting hwpoison multiple
times).

Well, for large folios (except hugetlb) that's not a problem, because we try
splitting to order-0 either way, and if that fails, we bail out.

So what remains is hugetlb, which is nasty.

I don't really enjoy having any special-casing here, because the moment we e.g.,
change how soft-offlining deals with splitting, we would be in trouble.

Assume we start support splitting to min-order at some point, we'd also want o
skip over min-order. Gah.


Can we just make our life easier and disallow specifying ranges for
MADV_HWPOISON/MADV_SOFT_OFFLINE?

It's a pure testing interface IIRC. tools/testing/selftests/mm/memory-failure.c
seems to always call it with PAGE_SIZE. Similarly
tools/testing/selftests/mm/hugetlb-read-hwpoison.c

There is one catch I think: an existing LTP test case issues MADV_SOFT_OFFLINE
on a larger range. But it respects -EINVAL at least :)

So we could return -EINVAL and fixup the test case to issue multiple madvise().

[1]
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/move_pages/move_pages12.c

-- 
Cheers,

David