Re: [PATCH] mm/sparse: Fix race on mem_section->usage in pfn walkers

[David Hildenbrand (Arm)]

On 4/15/26 11:20, Muchun Song wrote:
> 
> 
>> On Apr 15, 2026, at 16:04, David Hildenbrand (Arm) <david@xxxxxxxxxx> wrote:
>>
>> On 4/15/26 04:23, Muchun Song wrote:
>>> When memory is hot-removed, section_deactivate() can tear down
>>> mem_section->usage while concurrent pfn walkers still inspect the
>>> subsection map via pfn_section_valid() or pfn_section_first_valid().
>>
>> I'll note that it's all racy either way: someone checking pfn_valid() /
>> pfn_to_online_page() can race with concurrent unplug.
> 
> Agree. When I first saw the commit message for 5ec8e8ea8b77, I was curious
> because the goal of this commit was to fix an access issue with ms->usage.
> Looking at the race diagram, I realized that while this only addresses the
> ->usage access, subsequent accesses to struct page will still be problematic.
> It's just that the former issue happened to be triggered first in this specific
> commit.
> 
>>
>> We've known that for years; it's hard to fix; it never ever triggers :)
> 
> Glad to know my analysis wasn't off! It seems I've just stumbled upon a
> 'well-known secret' within the community. :)

Heh, yes.

> 
>>
>> So is this really worth it, when we should in fact, work on protecting
>> the users of pfn_valid() / pfn_to_online_page() with rcu or similar?
> 
> I am not sure if it is worth fixing, especially since I just realized the
> community has been aware of this issue for many years. If we do decide to
> fix it, I think the most straightforward approach would be to protect it
> using RCU, something like:
> 
> # the user side of pfn_to_online_page():
> 	rcu_read_lock(); 
> 	page = pfn_to_online_page();
> 	if (!get_page_unless_zero(page))
> 	    goto out_unlock;
> 	rcu_read_unlock();


Right, but we'd have to protect against the sections being marked as
offline as well here, though. So against a pure concurrent offline_pages().

If you're looking for a project, this is really one worth doing! :)


pfn_to_online_page() is more in need for protection than pfn_valid() I
think.

-- 
Cheers,

David