Re: [PATCH 2/2] arm64: mmu: use pagetable_alloc_nolock() while stop_machine()

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Brendan,

> On Mon Dec 15, 2025 at 10:06 AM UTC, Yeoreum Yun wrote:
> [snip]
> >> Overall I am feeling a bit uncomfortable about this use of _nolock, but
> >> I am also feeling pretty ignorant about PREEMPT_RT and also about this
> >> arm64 code, so I am hesitant to suggest alternatives, I hope someone
> >> else can offer some input here...
> >
> > I understand. However, as I mentioned earlier,
> > my main intention was to hear opinions specifically about memory contention.
> >
> > That said, if there is no memory contention,
> > I don’t think using the _nolock API is necessarily a bad approach.
>
>
> > In fact, I believe a bigger issue is that, under PREEMPT_RT,
> > code that uses the regular memory allocation APIs may give users the false impression
> > that those APIs are “safe to use,” even though they are not.
>
> Yeah, I share this concern. I would bet I have written code that's
> broken under PREEMPT_RT (luckily only in Google's kernel fork). The
> comment for GFP_ATOMIC says:
>
>  * %GFP_ATOMIC users can not sleep and need the allocation to succeed. A lower
>  * watermark is applied to allow access to "atomic reserves".
>  * The current implementation doesn't support NMI and few other strict
>  * non-preemptive contexts (e.g. raw_spin_lock). The same applies to %GFP_NOWAIT.
>
> It kinda sounds like it's supposed to be OK to use GFP_ATOMIC in a
> normal preempt_disable() context. So do you know exactly why it's
> invalid to use it in this stop_machine() context here? Maybe we need to
> update this comment.

In non-PREEMPT_RT configurations, this is fine to use.
However, in PREEMPT_RT, it should not be used because
spin_lock becomes a sleepable lock backed by an rt-mutex.

From Documentation/locking/locktypes.rst:

  The fact that PREEMPT_RT changes the lock category of spinlock_t and
  rwlock_t from spinning to sleeping.

As you know, all locks related to memory allocation
(e.g., zone_lock, PCP locks, etc.) use spin_lock,
which becomes sleepable under PREEMPT_RT.

The callback of stop_machine() is executed in a preemption-disabled context
(see cpu_stopper_thread()). In this context, if it fails to acquire a spinlock
during memory allocation,
the task would be able to go to sleep while preemption is disabled,
which is an obviously problematic situation.

> Or maybe actually we need to fix the allocator
> so that GFP_ATOMIC allocs are safe in this context?

I don’t think so, because GFP_ATOMIC can still be used by
IRQ threads in PREEMPT_RT. (If an IRQ handler is non-threaded
due to IRQF_NO_THREAD, then it cannot use it.)

Although the root cause appears to be that the memory allocator uses spin_lock(),
I believe the real issue is where such allocations are allowed to be used.
If we changed the lock to raw_spin_lock(),
this would introduce significant latency during memory allocation.
So I believe this is why the memory allocator continues to use spin_lock().

IOW, what I really want to ask is whether
general memory allocation/free operations are
permissible in a stop_machine() context
(I think nolock() can be allowable only).

Thanks

--
Sincerely,
Yeoreum Yun