Re: [PATCH 32/46] mm/memory_hotplug: avoid races in pfn_to_online_page() with dying pgmap

[Lance Yang <lance.yang@linux.dev>]

On 2026/10/28 03:41, David Hildenbrand (Arm) wrote:
>> LLM stumbled upon a possible race in kpage_read() ...
>>
>> Say the reader passes pfn_section_valid() for a device PFN in a section
>> shared with online RAM, gets preempted, and resumes after
>> pgmap_array_delete(). We'd then find no pgmap and return pfn_to_page(pfn),
>> even though the subsection is no longer valid ...
> 
> I think that's similar to our pfn_to_online_page() vs. memory offlining races,
> just slightly different. Getting these races down to 0 is pretty hard.
> 
> This patch here tries to do better and arguably almost looks like a cleanup.
> 
> I wouldn't worry about that one for now; it's extremely unlikely to hit. And it'

Ack.

>>
>> Could we take the memory hotplug read lock in kpage_read(), covering both
>> the lookup and the metadata reads? We can drop it before put_user(), once
>> we're done with the page.
>>
>> Maybe along these lines?
>>
>> ---8<---
>> diff --git a/fs/proc/page.c b/fs/proc/page.c
>> index 6aa99e42ec22..12bed36d76dc 100644
>> --- a/fs/proc/page.c
>> +++ b/fs/proc/page.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/hugetlb.h>
>>   #include <linux/memremap.h>
>>   #include <linux/memcontrol.h>
>> +#include <linux/memory_hotplug.h>
>>   #include <linux/mmu_notifier.h>
>>   #include <linux/page_idle.h>
>>   #include <linux/kernel-page-flags.h>
>> @@ -77,6 +78,8 @@ static ssize_t kpage_read(struct file *file, char __user *buf,
>>   	count = min_t(unsigned long, count, (max_dump_pfn * KPMSIZE) - src);
>>
>>   	while (count > 0) {
>> +		/* Prevent memory removal while inspecting the memmap. */
>> +		get_online_mems();
> get_online_mems() is the big hammer we try to avoid (or ideally remove at some
> point). And it's most pfn_to_online_page() users that are affected.
> 
> A seqcount in pfn_to_online_page() could detect concurrent pgmap removal and
> retry from the start. But I'd rather delay something like that.

Fair enough, let's leave it alone for now given how unlikely it is to hit :)

> Thanks for taking a look! (Can I convince you to review some of the THP fixes?
> pleeeeaaasseee :) )

Of coursssssssse! I'm already looking through the THP fixes :D

Cheers, Lance