Re: [PATCH 0/2] introduce pagetable_alloc_nolock()

[Ryan Roberts <ryan.roberts@arm.com>]

On 16/12/2025 16:52, Yeoreum Yun wrote:
> Hi Ryan,
> 
>> On 12/12/2025 16:18, Yeoreum Yun wrote:
>>> Some architectures invoke pagetable_alloc() or __get_free_pages()
>>> with preemption disabled.
>>> For example, in arm64, linear_map_split_to_ptes() calls pagetable_alloc()
>>> while spliting block entry to ptes and __kpti_install_ng_mappings()
>>> calls __get_free_pages() to create kpti pagetable.
>>>
>>> Under PREEMPT_RT, calling pagetable_alloc() with
>>> preemption disabled is not allowed, because it may acquire
>>> a spin lock that becomes sleepable on RT, potentially
>>> causing a sleep during page allocation.
>>>
>>> Since above two functions is called as callback of stop_machine()
>>> where its callback is called in preemption disabled,
>>> They could make a potential problem. (sleeping in preemption disabled).
>>>
>>> To address this, introduce pagetable_alloc_nolock() API.
>>
>> I don't really understand what the problem is that you're trying to fix. As I
>> see it, there are 2 call sites in arm64 arch code that are calling into the page
>> allocator from stop_machine() - one via via pagetable_alloc() and another via
>> __get_free_pages(). But both of those calls are passing in GFP_ATOMIC. It was my
>> understanding that the page allocator would ensure it never sleeps when
>> GFP_ATOMIC is passed in, (even for PREEMPT_RT)?
> 
> Although GFP_ATOMIC is specify, it only affects of "water mark" of the
> page with __GFP_HIGH. and to get a page, it must grab the lock --
> zone->lock or pcp_lock in the rmqueue().
> 
> This zone->lock and pcp_lock is spin_lock and it's a sleepable in
> PREEMPT_RT that's why the memory allocation/free using general API
> except nolock() version couldn't be called since
> if "contention" happens they'll sleep while waiting to get the lock.
> 
> The reason why "nolock()" can use, it always uses "trylock" with
> ALLOC_TRYLOCK flags. otherwise GFP_ATOMIC also can be sleepable in
> PREEMPT_RT.
> 
>>
>> What is the actual symptom you are seeing?
> 
> Since the place where called while smp_cpus_done() and there seems no
> contention, there seems no problem. However as I mention in another
> thread
> (https://lore.kernel.org/all/aT%2FdrjN1BkvyAGoi@e129823.arm.com/),
> This gives a the false impression --
> GFP_ATOMIC are “safe to use in preemption disabled”
> even though they are not in PREEMPT_RT case, I've changed it.
> 
>>
>> If the page allocator is somehow ignoring the GFP_ATOMIC request for PREEMPT_RT,
>> then isn't that a bug in the page allocator? I'm not sure why you would change
>> the callsites? Can't you just change the page allocator based on GFP_ATOMIC?
> 
> It doesn't ignore the GFP_ATOMIC feature:
>   - __GFP_HIGH: use water mark till min reserved
>   - __GFP_KSWAPD_RECLAIM: wake up kswapd if reclaim required.
> 
> But, it's a restriction -- "page allocation / free" API cannot be called
> in preempt-disabled context at PREEMPT_RT.
> 
> That's why I think it's wrong usage not a page allocator bug.

I've taken a look at this and I agree with your analysis. Thanks for explaining.

Looking at other stop_machine() callbacks, there are some that call printk() and
I would assume that spinlocks could be taken there which may present the same
kind of issue or PREEMPT_RT? (I'm guessing). I don't see any others that attempt
to allocate memory though.

Anyway, to fix the 2 arm64 callsites, I see 2 possible approaches:

- Call the nolock variant (as you are doing). But that would just convert a
deadlock to a panic; if the lock is held when stop_machine() runs, without your
change, we now have a deadlock due to waiting on the lock inside stop_machine().
With your change, we notice the lock is already taken and panic. I guess it is
marginally better, but not by much. Certainly I would just _always_ call the
nolock variant regardless of PREEMPT_RT if we take this route; For !PREEMPT_RT,
the lock is guarranteed to be free so nolock will always succeed.

- Preallocate the memory before entering stop_machine(). I think this would be
much more robust. For kpti_install_ng_mappings() I think you could hoist the
allocation/free out of stop_machine() and pass the pointer in pretty easily. For
linear_map_split_to_ptes() its a bit more complex; Perhaps, we need to walk the
pgtable to figure out how much to preallocate, allocate it, then set it up as a
special allocator, wrapped by an allocation function and modify the callchain to
take a callback function instead of gfp flags.

What do you think?

Thanks,
Ryan

> 
> [...]
> 
> --
> Sincerely,
> Yeoreum Yun