[PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit

Zi Yan posted 4 patches 7 months, 1 week ago
There is a newer version of this series
drivers/virtio/virtio_mem.c     |   3 +-
include/linux/gfp.h             |   6 +-
include/linux/mmzone.h          |  17 +++--
include/linux/page-isolation.h  |  33 +++++++--
include/linux/pageblock-flags.h |   9 ++-
include/trace/events/kmem.h     |  14 ++--
mm/cma.c                        |   2 +-
mm/memory_hotplug.c             |  14 ++--
mm/page_alloc.c                 | 126 ++++++++++++++++++++++++--------
mm/page_isolation.c             | 114 +++++++++++++++--------------
10 files changed, 226 insertions(+), 112 deletions(-)
[PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months, 1 week ago
Hi David and Oscar,

Can you take a look at Patch 2, which changes how online_pages() set
online pageblock migratetypes? It used to first set all pageblocks to
MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
Let me know if there is any issue with my changes.

Hi Johannes,

Patch 2 now have set_pageblock_migratetype() not accepting
MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
feedback.

Hi all,

This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
being overwritten during pageblock isolation process. Currently,
MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
migratetype. This causes pageblock migratetype loss during
alloc_contig_range() and memory offline, especially when the process
fails due to a failed pageblock isolation and the code tries to undo the
finished pageblock isolations.

It is on top of mm-everything-2025-05-09-01-57 with v3 reverted.

In terms of performance for changing pageblock types, no performance
change is observed:

1. I used perf to collect stats of offlining and onlining all memory of a
40GB VM 10 times and see that get_pfnblock_flags_mask() and
set_pfnblock_flags_mask() take about 0.12% and 0.02% of the whole process
respectively with and without this patchset across 3 runs.

2. I used perf to collect stats of dd from /dev/random to a 40GB tmpfs file
and find get_pfnblock_flags_mask() takes about 0.05% of the process with and
without this patchset across 3 runs.


Changelog
===
From v3[2]:
1. kept the original is_migrate_isolate_page()
2. moved {get,set,clear}_pageblock_isolate() to mm/page_isolation.c
3. used a single version for get_pageblock_migratetype() and
   get_pfnblock_migratetype().
4. replace get_pageblock_isolate() with
   get_pageblock_migratetype() == MIGRATE_ISOLATE, a
   get_pageblock_isolate() becomes private in mm/page_isolation.c
5. made set_pageblock_migratetype() not accept MIGRATE_ISOLATE, so that
   people need to use the dedicate {get,set,clear}_pageblock_isolate() APIs.
6. changed online_page() from mm/memory_hotplug.c to first set pageblock
   migratetype to MIGRATE_MOVABLE, then isolate pageblocks.
7. added __maybe_unused to get_pageblock_isolate(), since it is only
   used in VM_BUG_ON(), which could be not present when MM debug is off.
   It is reported by kernel test robot.
7. fixed test_pages_isolated() type issues reported by kernel test
   robot.

From v2[1]:
1. Moved MIGRATETYPE_NO_ISO_MASK to Patch 2, where it is used.
2. Removed spurious changes in Patch 1.
3. Refactored code so that migratetype mask is passed properly for all
callers to {get,set}_pfnblock_flags_mask().
4. Added toggle_pageblock_isolate() for setting and clearing
MIGRATE_ISOLATE.
5. Changed get_pageblock_migratetype() when CONFIG_MEMORY_ISOLATION to
handle MIGRATE_ISOLATE case. It acts like a parsing layer for
get_pfnblock_flags_mask().


Design
===

Pageblock flags are read in words to achieve good performance and existing
pageblock flags take 4 bits per pageblock. To avoid a substantial change
to the pageblock flag code, pageblock flag bits are expanded to use 8
and MIGRATE_ISOLATE is moved to use the last bit (bit 7).

It might look like the pageblock flags have doubled the overhead, but in
reality, the overhead is only 1 byte per 2MB/4MB (based on pageblock config),
or 0.0000476 %.

Any comment and/or suggestion is welcome. Thanks.

[1] https://lore.kernel.org/linux-mm/20250214154215.717537-1-ziy@nvidia.com/
[2] https://lore.kernel.org/linux-mm/20250507211059.2211628-2-ziy@nvidia.com/


Zi Yan (4):
  mm/page_isolation: make page isolation a standalone bit.
  mm/page_isolation: remove migratetype from
    move_freepages_block_isolate()
  mm/page_isolation: remove migratetype from undo_isolate_page_range()
  mm/page_isolation: remove migratetype parameter from more functions.

 drivers/virtio/virtio_mem.c     |   3 +-
 include/linux/gfp.h             |   6 +-
 include/linux/mmzone.h          |  17 +++--
 include/linux/page-isolation.h  |  33 +++++++--
 include/linux/pageblock-flags.h |   9 ++-
 include/trace/events/kmem.h     |  14 ++--
 mm/cma.c                        |   2 +-
 mm/memory_hotplug.c             |  14 ++--
 mm/page_alloc.c                 | 126 ++++++++++++++++++++++++--------
 mm/page_isolation.c             | 114 +++++++++++++++--------------
 10 files changed, 226 insertions(+), 112 deletions(-)

-- 
2.47.2
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by David Hildenbrand 7 months ago
On 09.05.25 22:01, Zi Yan wrote:
> Hi David and Oscar,

Hi,

> 
> Can you take a look at Patch 2, which changes how online_pages() set
> online pageblock migratetypes?

Sorry, now looking :)

> It used to first set all pageblocks to
> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
> Let me know if there is any issue with my changes.

Conceptually, we should start with MIGRATE_MOVABLE + isolated, to then 
clear the isolated bit.

Let me take a look.


-- 
Cheers,

David / dhildenb
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 19 May 2025, at 3:44, David Hildenbrand wrote:

> On 09.05.25 22:01, Zi Yan wrote:
>> Hi David and Oscar,
>
> Hi,
>
>>
>> Can you take a look at Patch 2, which changes how online_pages() set
>> online pageblock migratetypes?
>
> Sorry, now looking :)
>
>> It used to first set all pageblocks to
>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>> Let me know if there is any issue with my changes.
>
> Conceptually, we should start with MIGRATE_MOVABLE + isolated, to then clear the isolated bit.

OK, in my current V5, I added
void init_pageblock_migratetype(struct page, int migratetype, bool isolate) to
do this, so that one can initialize a pageblock with a migratetype + isolated or not.
Let me check your comments on Patch 4 too.


--
Best Regards,
Yan, Zi
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Vlastimil Babka 7 months ago
On 5/9/25 22:01, Zi Yan wrote:
> Hi David and Oscar,
> 
> Can you take a look at Patch 2, which changes how online_pages() set
> online pageblock migratetypes? It used to first set all pageblocks to
> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
> Let me know if there is any issue with my changes.
> 
> Hi Johannes,
> 
> Patch 2 now have set_pageblock_migratetype() not accepting
> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
> feedback.
> 
> Hi all,
> 
> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
> being overwritten during pageblock isolation process. Currently,
> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
> migratetype. This causes pageblock migratetype loss during
> alloc_contig_range() and memory offline, especially when the process
> fails due to a failed pageblock isolation and the code tries to undo the
> finished pageblock isolations.

Seems mostly fine to me, just sent suggestion for 4/4.
I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
I also see that it's useful for it to be because then it means it has the
freelists in the buddy allocator, can work via __move_freepages_block() etc.
Oh well. So it's still a migratetype, but the pageblock handling is now
different.
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 17 May 2025, at 16:26, Vlastimil Babka wrote:

> On 5/9/25 22:01, Zi Yan wrote:
>> Hi David and Oscar,
>>
>> Can you take a look at Patch 2, which changes how online_pages() set
>> online pageblock migratetypes? It used to first set all pageblocks to
>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>> Let me know if there is any issue with my changes.
>>
>> Hi Johannes,
>>
>> Patch 2 now have set_pageblock_migratetype() not accepting
>> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
>> feedback.
>>
>> Hi all,
>>
>> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
>> being overwritten during pageblock isolation process. Currently,
>> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
>> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
>> migratetype. This causes pageblock migratetype loss during
>> alloc_contig_range() and memory offline, especially when the process
>> fails due to a failed pageblock isolation and the code tries to undo the
>> finished pageblock isolations.
>
> Seems mostly fine to me, just sent suggestion for 4/4.

Thanks.

> I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
> I also see that it's useful for it to be because then it means it has the
> freelists in the buddy allocator, can work via __move_freepages_block() etc.

Yeah, I wanted to remove MIGRATE_ISOLATE from migratetype too, but there
is a MIGRATE_ISOLATE freelist and /proc/pagetypeinfo also shows isolated
free pages.

> Oh well. So it's still a migratetype, but the pageblock handling is now
> different.

Yep. We also have PB_migrate_skip, a bit in pageblock_bits used for memory
compaction and not part of migratetype.

--
Best Regards,
Yan, Zi
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by David Hildenbrand 7 months ago
On 18.05.25 02:20, Zi Yan wrote:
> On 17 May 2025, at 16:26, Vlastimil Babka wrote:
> 
>> On 5/9/25 22:01, Zi Yan wrote:
>>> Hi David and Oscar,
>>>
>>> Can you take a look at Patch 2, which changes how online_pages() set
>>> online pageblock migratetypes? It used to first set all pageblocks to
>>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>>> Let me know if there is any issue with my changes.
>>>
>>> Hi Johannes,
>>>
>>> Patch 2 now have set_pageblock_migratetype() not accepting
>>> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
>>> feedback.
>>>
>>> Hi all,
>>>
>>> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
>>> being overwritten during pageblock isolation process. Currently,
>>> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
>>> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
>>> migratetype. This causes pageblock migratetype loss during
>>> alloc_contig_range() and memory offline, especially when the process
>>> fails due to a failed pageblock isolation and the code tries to undo the
>>> finished pageblock isolations.
>>
>> Seems mostly fine to me, just sent suggestion for 4/4.
> 
> Thanks.
> 
>> I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
>> I also see that it's useful for it to be because then it means it has the
>> freelists in the buddy allocator, can work via __move_freepages_block() etc.
> 
> Yeah, I wanted to remove MIGRATE_ISOLATE from migratetype too, but there
> is a MIGRATE_ISOLATE freelist and /proc/pagetypeinfo also shows isolated
> free pages.

The latter, we can likely fake.

Is there a reasonable way to remove MIGRATE_ISOLATE completely?

Of course, we could simply duplicate the page lists (one set for 
isolated, one set for !isolated), or keep it as is and simply have a
separate one that we separate out. So, we could have a 
migratetype+isolated pair instead.

Just a thought, did not look into all the ugly details.

-- 
Cheers,

David / dhildenb
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 19 May 2025, at 10:15, David Hildenbrand wrote:

> On 18.05.25 02:20, Zi Yan wrote:
>> On 17 May 2025, at 16:26, Vlastimil Babka wrote:
>>
>>> On 5/9/25 22:01, Zi Yan wrote:
>>>> Hi David and Oscar,
>>>>
>>>> Can you take a look at Patch 2, which changes how online_pages() set
>>>> online pageblock migratetypes? It used to first set all pageblocks to
>>>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>>>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>>>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>>>> Let me know if there is any issue with my changes.
>>>>
>>>> Hi Johannes,
>>>>
>>>> Patch 2 now have set_pageblock_migratetype() not accepting
>>>> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
>>>> feedback.
>>>>
>>>> Hi all,
>>>>
>>>> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
>>>> being overwritten during pageblock isolation process. Currently,
>>>> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
>>>> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
>>>> migratetype. This causes pageblock migratetype loss during
>>>> alloc_contig_range() and memory offline, especially when the process
>>>> fails due to a failed pageblock isolation and the code tries to undo the
>>>> finished pageblock isolations.
>>>
>>> Seems mostly fine to me, just sent suggestion for 4/4.
>>
>> Thanks.
>>
>>> I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
>>> I also see that it's useful for it to be because then it means it has the
>>> freelists in the buddy allocator, can work via __move_freepages_block() etc.
>>
>> Yeah, I wanted to remove MIGRATE_ISOLATE from migratetype too, but there
>> is a MIGRATE_ISOLATE freelist and /proc/pagetypeinfo also shows isolated
>> free pages.
>
> The latter, we can likely fake.
>
> Is there a reasonable way to remove MIGRATE_ISOLATE completely?
>
> Of course, we could simply duplicate the page lists (one set for isolated, one set for !isolated), or keep it as is and simply have a

That could work. It will change vmcore layout and I wonder if that is a concern
or not.

> separate one that we separate out. So, we could have a migratetype+isolated pair instead.

What do you mean by a migratetype+isolate pair?

>
> Just a thought, did not look into all the ugly details.

Another thought is that maybe caller should keep the isolated free pages instead
to make it actually isolated. We might need to keep per-order isolated free page
stats to fake /proc/pagetypeinfo.

--
Best Regards,
Yan, Zi
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by David Hildenbrand 7 months ago
On 19.05.25 16:35, Zi Yan wrote:
> On 19 May 2025, at 10:15, David Hildenbrand wrote:
> 
>> On 18.05.25 02:20, Zi Yan wrote:
>>> On 17 May 2025, at 16:26, Vlastimil Babka wrote:
>>>
>>>> On 5/9/25 22:01, Zi Yan wrote:
>>>>> Hi David and Oscar,
>>>>>
>>>>> Can you take a look at Patch 2, which changes how online_pages() set
>>>>> online pageblock migratetypes? It used to first set all pageblocks to
>>>>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>>>>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>>>>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>>>>> Let me know if there is any issue with my changes.
>>>>>
>>>>> Hi Johannes,
>>>>>
>>>>> Patch 2 now have set_pageblock_migratetype() not accepting
>>>>> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
>>>>> feedback.
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
>>>>> being overwritten during pageblock isolation process. Currently,
>>>>> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
>>>>> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
>>>>> migratetype. This causes pageblock migratetype loss during
>>>>> alloc_contig_range() and memory offline, especially when the process
>>>>> fails due to a failed pageblock isolation and the code tries to undo the
>>>>> finished pageblock isolations.
>>>>
>>>> Seems mostly fine to me, just sent suggestion for 4/4.
>>>
>>> Thanks.
>>>
>>>> I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
>>>> I also see that it's useful for it to be because then it means it has the
>>>> freelists in the buddy allocator, can work via __move_freepages_block() etc.
>>>
>>> Yeah, I wanted to remove MIGRATE_ISOLATE from migratetype too, but there
>>> is a MIGRATE_ISOLATE freelist and /proc/pagetypeinfo also shows isolated
>>> free pages.
>>
>> The latter, we can likely fake.
>>
>> Is there a reasonable way to remove MIGRATE_ISOLATE completely?
>>
>> Of course, we could simply duplicate the page lists (one set for isolated, one set for !isolated), or keep it as is and simply have a
> 
> That could work. It will change vmcore layout and I wonder if that is a concern
> or not.

Not really. makedumpfile will have to implement support for the new 
layout as it adds support for the new kernel version.

> 
>> separate one that we separate out. So, we could have a migratetype+isolated pair instead.
> 
> What do you mean by a migratetype+isolate pair?

If MIGRATE_ISOLATE no longer exists, relevant code would have to pass 
migratetype+isolated (essentially, what you did in 
init_pageblock_migratetype ).


E.g., we could pass around a "pageblock_info" (or however we call it, 
using a different type than a bare migratetype) from which we can easily 
extract the migratetype and the isolated state.


E.g., init_pageblock_migratetype() could then become

struct pageblock_info pb_info = {
	.migratetype = MIGRATE_MOVABLE,
	.isolated = true,
}
init_pageblock_info(page, pb_info);

So, we'd decouple the migratetype we pass around from the "isolated" 
state. Whoever needs the "isolated" state in addition to the migratetype 
should use get_pageblock_info().

When adding to lists, we can decide what to do based on that information.

> 
>>
>> Just a thought, did not look into all the ugly details.
> 
> Another thought is that maybe caller should keep the isolated free pages instead
> to make it actually isolated.

You mean, not adding them to a list at all in the buddy? I think the 
problem is that if a page gets freed while the pageblock is isolated, it 
cannot get added to the list of an owner easily.

-- 
Cheers,

David / dhildenb
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 20 May 2025, at 4:58, David Hildenbrand wrote:

> On 19.05.25 16:35, Zi Yan wrote:
>> On 19 May 2025, at 10:15, David Hildenbrand wrote:
>>
>>> On 18.05.25 02:20, Zi Yan wrote:
>>>> On 17 May 2025, at 16:26, Vlastimil Babka wrote:
>>>>
>>>>> On 5/9/25 22:01, Zi Yan wrote:
>>>>>> Hi David and Oscar,
>>>>>>
>>>>>> Can you take a look at Patch 2, which changes how online_pages() set
>>>>>> online pageblock migratetypes? It used to first set all pageblocks to
>>>>>> MIGRATE_ISOLATE, then let undo_isolate_page_range() move the pageblocks
>>>>>> to MIGRATE_MOVABLE. After MIGRATE_ISOLATE becomes a standalone bit, all
>>>>>> online pageblocks need to have a migratetype other than MIGRATE_ISOLATE.
>>>>>> Let me know if there is any issue with my changes.
>>>>>>
>>>>>> Hi Johannes,
>>>>>>
>>>>>> Patch 2 now have set_pageblock_migratetype() not accepting
>>>>>> MIGRATE_ISOLATE. I think it makes code better. Thank you for the great
>>>>>> feedback.
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
>>>>>> being overwritten during pageblock isolation process. Currently,
>>>>>> MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
>>>>>> thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
>>>>>> migratetype. This causes pageblock migratetype loss during
>>>>>> alloc_contig_range() and memory offline, especially when the process
>>>>>> fails due to a failed pageblock isolation and the code tries to undo the
>>>>>> finished pageblock isolations.
>>>>>
>>>>> Seems mostly fine to me, just sent suggestion for 4/4.
>>>>
>>>> Thanks.
>>>>
>>>>> I was kinda hoping that MIGRATE_ISOLATE could stop being a migratetype. But
>>>>> I also see that it's useful for it to be because then it means it has the
>>>>> freelists in the buddy allocator, can work via __move_freepages_block() etc.
>>>>
>>>> Yeah, I wanted to remove MIGRATE_ISOLATE from migratetype too, but there
>>>> is a MIGRATE_ISOLATE freelist and /proc/pagetypeinfo also shows isolated
>>>> free pages.
>>>
>>> The latter, we can likely fake.
>>>
>>> Is there a reasonable way to remove MIGRATE_ISOLATE completely?
>>>
>>> Of course, we could simply duplicate the page lists (one set for isolated, one set for !isolated), or keep it as is and simply have a
>>
>> That could work. It will change vmcore layout and I wonder if that is a concern
>> or not.
>
> Not really. makedumpfile will have to implement support for the new layout as it adds support for the new kernel version.

Got it.

>
>>
>>> separate one that we separate out. So, we could have a migratetype+isolated pair instead.
>>
>> What do you mean by a migratetype+isolate pair?
>
> If MIGRATE_ISOLATE no longer exists, relevant code would have to pass migratetype+isolated (essentially, what you did in init_pageblock_migratetype ).
>
>
> E.g., we could pass around a "pageblock_info" (or however we call it, using a different type than a bare migratetype) from which we can easily extract the migratetype and the isolated state.
>
>
> E.g., init_pageblock_migratetype() could then become
>
> struct pageblock_info pb_info = {
> 	.migratetype = MIGRATE_MOVABLE,
> 	.isolated = true,
> }
> init_pageblock_info(page, pb_info);
>
> So, we'd decouple the migratetype we pass around from the "isolated" state. Whoever needs the "isolated" state in addition to the migratetype should use get_pageblock_info().
>
> When adding to lists, we can decide what to do based on that information.

This looks good to me. I can send a follow-up patchset to get rid of
MIGRATE_ISOLATE along with more cleanups like changing "int migratetype" to
"enum migratetype migratetype" in mm/page_alloc.c.

>
>>
>>>
>>> Just a thought, did not look into all the ugly details.
>>
>> Another thought is that maybe caller should keep the isolated free pages instead
>> to make it actually isolated.
>
> You mean, not adding them to a list at all in the buddy? I think the problem is that
Yes.
> if a page gets freed while the pageblock is isolated, it cannot get added to the list of an owner easily.

Right. In theory, it is possible, since when a MIGRATED_ISOLATE page is freed,
__free_one_page() can find its buddy and add the freed page to its buddy's
buddy_list without performing a merge like current code. But it needs a new
code path in __add_to_free_list(), since it is not added to the head nor the
tail of a free list.

--
Best Regards,
Yan, Zi
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by David Hildenbrand 7 months ago
>> if a page gets freed while the pageblock is isolated, it cannot get added to the list of an owner easily.
> 
> Right. In theory, it is possible, since when a MIGRATED_ISOLATE page is freed,
> __free_one_page() can find its buddy and add the freed page to its buddy's
> buddy_list without performing a merge like current code. But it needs a new
> code path in __add_to_free_list(), since it is not added to the head nor the
> tail of a free list.

But what if the whole pageblock gets freed in a single shot (IOW, there 
is no buddy to lookup the list for?).

-- 
Cheers,

David / dhildenb
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 20 May 2025, at 9:20, David Hildenbrand wrote:

>>> if a page gets freed while the pageblock is isolated, it cannot get added to the list of an owner easily.
>>
>> Right. In theory, it is possible, since when a MIGRATED_ISOLATE page is freed,
>> __free_one_page() can find its buddy and add the freed page to its buddy's
>> buddy_list without performing a merge like current code. But it needs a new
>> code path in __add_to_free_list(), since it is not added to the head nor the
>> tail of a free list.
>
> But what if the whole pageblock gets freed in a single shot (IOW, there is no buddy to lookup the list for?).

You are right. This means when MIGRATE_ISOLATE is removed, the global
MIGRATE_ISOLATE free list stays. BTW, looking at struct free_area,
its nr_free accounts for free pages in all migratetypes. Either struct free_area
stays the same or more code changes are needed to have isolated free list
+ !isolated free list. I will try to figure this out in the follow-up patchset.

--
Best Regards,
Yan, Zi
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by David Hildenbrand 7 months ago
On 20.05.25 15:31, Zi Yan wrote:
> On 20 May 2025, at 9:20, David Hildenbrand wrote:
> 
>>>> if a page gets freed while the pageblock is isolated, it cannot get added to the list of an owner easily.
>>>
>>> Right. In theory, it is possible, since when a MIGRATED_ISOLATE page is freed,
>>> __free_one_page() can find its buddy and add the freed page to its buddy's
>>> buddy_list without performing a merge like current code. But it needs a new
>>> code path in __add_to_free_list(), since it is not added to the head nor the
>>> tail of a free list.
>>
>> But what if the whole pageblock gets freed in a single shot (IOW, there is no buddy to lookup the list for?).
> 

And thinking about it, another problem is if a page gets freed that has 
no buddies.

> You are right. This means when MIGRATE_ISOLATE is removed, the global
> MIGRATE_ISOLATE free list stays.

Right. We could just have that one separately from the existing array.

-- 
Cheers,

David / dhildenb
Re: [PATCH v4 0/4] Make MIGRATE_ISOLATE a standalone bit
Posted by Zi Yan 7 months ago
On 20 May 2025, at 9:33, David Hildenbrand wrote:

> On 20.05.25 15:31, Zi Yan wrote:
>> On 20 May 2025, at 9:20, David Hildenbrand wrote:
>>
>>>>> if a page gets freed while the pageblock is isolated, it cannot get added to the list of an owner easily.
>>>>
>>>> Right. In theory, it is possible, since when a MIGRATED_ISOLATE page is freed,
>>>> __free_one_page() can find its buddy and add the freed page to its buddy's
>>>> buddy_list without performing a merge like current code. But it needs a new
>>>> code path in __add_to_free_list(), since it is not added to the head nor the
>>>> tail of a free list.
>>>
>>> But what if the whole pageblock gets freed in a single shot (IOW, there is no buddy to lookup the list for?).
>>
>
> And thinking about it, another problem is if a page gets freed that has no buddies.
>
>> You are right. This means when MIGRATE_ISOLATE is removed, the global
>> MIGRATE_ISOLATE free list stays.
>
> Right. We could just have that one separately from the existing array.

Yep,

struct free_area {
	struct list_head	free_list[MIGRATE_TYPES];
	struct list_head	isolate_list;
	unsigned long		nr_free;
};

--
Best Regards,
Yan, Zi