Re: [PATCH v2] mm/page_alloc: Add lockdep assertion for pageblock type change

[David Hildenbrand]

On 03.03.25 13:13, Brendan Jackman wrote:
> Since the migratetype hygiene patches [0], the locking here is
> a bit more formalised.
>
> For other stuff, it's pretty obvious that it would be protected by the
> zone lock. But it didn't seem totally self-evident that it should
> protect the pageblock type. So it seems particularly helpful to have it
> written in the code.

[...]

> +
>   u64 max_mem_size = U64_MAX;
>   
>   /* add this memory to iomem resource */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 579789600a3c7bfb7b0d847d51af702a9d4b139a..1ed21179676d05c66f77f9dbebf88e36bbe402e9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -417,6 +417,10 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
>   
>   void set_pageblock_migratetype(struct page *page, int migratetype)
>   {
> +	lockdep_assert_once(system_state == SYSTEM_BOOTING ||
> +		in_mem_hotplug() ||
> +		lockdep_is_held(&page_zone(page)->lock));
> +

I assume the call chain on the memory hotplug path is mostly

move_pfn_range_to_zone()->memmap_init_range()->set_pageblock_migratetype()

either when onlining a memory block, or from pagemap_range() while 
holding the hotplug lock.

But there is also the 
memmap_init_zone_device()->memmap_init_compound()->__init_zone_device_page()->set_pageblock_migratetype() 
one, called from pagemap_range() *without* holding the hotplug lock, and 
you assertion would be missing that.

I'm not too happy about that assertion in general.

--
Cheers,

David / dhildenb