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

[David Hildenbrand <david@redhat.com>]

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>


-- 
Cheers,

David / dhildenb