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

[David Hildenbrand (Arm)]

> 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'

> 
> 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.

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

-- 
Cheers,

David