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

David Hildenbrand
2025-03-03T13:48:39Z

On 03.03.25 14:11, David Hildenbrand wrote:
> 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.

Heh, and I even ran into that right now by accident during boot:

[ 9.790696][ T1] WARNING: CPU: 3 PID: 1 at mm/page_alloc.c:420 set_pageblock_migratetype+0xb3/0xf0
[ 9.792672][ T1] Modules linked in:
[ 9.793496][ T1] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc3-00349-geaddff2b220c #164
[ 9.795511][ T1] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40 04/01/2014
[ 9.797471][ T1] RIP: 0010:set_pageblock_migratetype+0xb3/0xf0
[ 9.798794][ T1] Code: 2c c5 c0 0b 6d 91 73 43 4d 69 e4 40 07 00 00 be ff ff ff ff 4b 8d bc 25 18 06 00 00 e8 46 09 1b 04 85 c0 0f 85 71 ff ff ff 90 <0f> 0b 90 e9 68 ff ff ff 31 db e9 74 ff ff ff 48 c7 c6 58 4a f7 86
[ 9.802906][ T1] RSP: 0018:ffffc9000013bcc0 EFLAGS: 00010246
[ 9.804198][ T1] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000001
[ 9.805860][ T1] RDX: 0000000000000046 RSI: ffffffff8725b542 RDI: ffffffff872bd087
[ 9.807528][ T1] RBP: ffffeaffffc00000 R08: 0000000000000005 R09: 0000000000000000
[ 9.809186][ T1] R10: 0000000000000000 R11: 0000000000000002 R12: 0000000000001d00
[ 9.810871][ T1] R13: ffff88847fffa540 R14: 0000000000000000 R15: 0000000000000000
[ 9.812526][ T1] FS: 0000000000000000(0000) GS:ffff88846fcc0000(0000) knlGS:0000000000000000
[ 9.814388][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9.815751][ T1] CR2: 0000000000000000 CR3: 00000000076d8000 CR4: 0000000000750ef0
[ 9.817410][ T1] PKRU: 55555554
[ 9.818143][ T1] Call Trace:
[ 9.818821][ T1] <TASK>
[ 9.819428][ T1] ? set_pageblock_migratetype+0xb3/0xf0
[ 9.820639][ T1] ? __warn.cold+0x110/0x210
[ 9.821618][ T1] ? set_pageblock_migratetype+0xb3/0xf0
[ 9.822809][ T1] ? report_bug+0x1b9/0x320
[ 9.823762][ T1] ? handle_bug+0x54/0x90
[ 9.824675][ T1] ? exc_invalid_op+0x17/0x50
[ 9.825659][ T1] ? asm_exc_invalid_op+0x1a/0x20
[ 9.826727][ T1] ? set_pageblock_migratetype+0xb3/0xf0
[ 9.827914][ T1] __init_zone_device_page.constprop.0+0x20c/0x240
[ 9.829293][ T1] memmap_init_zone_device+0x191/0x330
[ 9.830478][ T1] memremap_pages+0x4b7/0xc80
[ 9.831485][ T1] dmirror_allocate_chunk+0x12b/0x400
[ 9.832628][ T1] hmm_dmirror_init+0x18f/0x260
[ 9.833657][ T1] ? __pfx_hmm_dmirror_init+0x10/0x10
[ 9.834798][ T1] do_one_initcall+0xa5/0x490
[ 9.835789][ T1] kernel_init_freeable+0x3b4/0x410
[ 9.836897][ T1] ? __pfx_kernel_init+0x10/0x10
[ 9.837938][ T1] kernel_init+0x1b/0x1d0
[ 9.838856][ T1] ret_from_fork+0x48/0x60
[ 9.839796][ T1] ? __pfx_kernel_init+0x10/0x10
[ 9.840864][ T1] ret_from_fork_asm+0x1a/0x30
[ 9.841878][ T1] </TASK>