On Mon, Mar 03, 2025 at 06:06:12PM +0100, David Hildenbrand wrote:
> > > But I am not sure why memmap_init_zone_device() would have to set the
> > > migratetype at all? Because migratetype is a buddy concept, and
> > > ZONE_DEVICE does not interact with the buddy to that degree.
> > >
> > > The comment in __init_zone_device_page states:
> > >
> > > "Mark the block movable so that blocks are reserved for movable at
> > > startup. This will force kernel allocations to reserve their blocks
> > > rather than leaking throughout the address space during boot when
> > > many long-lived kernel allocations are made."
> >
> > Uh, yeah I was pretty mystified by that. It would certainly be nice if
> > we can just get rid of this modification path.
> >
> > > But that just dates back to 966cf44f637e where we copy-pasted that code.
> > >
> > > So I wonder if we could just
> > >
> > > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > > index 57933683ed0d1..b95f545846e6e 100644
> > > --- a/mm/mm_init.c
> > > +++ b/mm/mm_init.c
> > > @@ -1002,19 +1002,11 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
> > > page->zone_device_data = NULL;
> > > /*
> > > - * Mark the block movable so that blocks are reserved for
> > > - * movable at startup. This will force kernel allocations
> > > - * to reserve their blocks rather than leaking throughout
> > > - * the address space during boot when many long-lived
> > > - * kernel allocations are made.
> > > - *
> > > - * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
> > > - * because this is done early in section_activate()
> > > + * Note that we leave pageblock migratetypes uninitialized, because
> > > + * they don't apply to ZONE_DEVICE.
> > > */
> > > - if (pageblock_aligned(pfn)) {
> > > - set_pageblock_migratetype(page, MIGRATE_MOVABLE);
> > > + if (pageblock_aligned(pfn))
> > > cond_resched();
> > > - }
> > > /*
> > > * ZONE_DEVICE pages other than MEMORY_TYPE_GENERIC are released
> >
> > memory-model.rst says:
> >
> > > Since the
> > > page reference count never drops below 1 the page is never tracked as
> > > free memory and the page's `struct list_head lru` space is repurposed
> > > for back referencing to the host device / driver that mapped the memory.
>
> That comment will be stale soon. In general, ZONE_DEVICE refcounts can drop
> to 0, but they will never go to the buddy, but will get intercepted on the
> freeing path.
>
> >
> > And this code seems to assume that the whole pageblock is part of the
> > ZONE_DEVICE dance, it would certainly make sense to me...
>
> Sorry, I didn't get your final conclusion: do you thing we don't have to
> initialize the migratetype, or do you think there is reason to still do it?
Sorry yeah, I was concluding that I don't see any reason to set the
migratetype here. But, given I didn't even notice this code path until
your review here I am not feeling too confident about changing it.
I can try to stare it some more and hopefully some courage will build
up! I probably also need to find a way to run a system that uses
ZONE_DEVICE for my local testing...
