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

On Tue Dec 16, 2025 at 12:01 PM UTC, Yeoreum Yun wrote:

quoted

On Tue Dec 16, 2025 at 11:03 AM UTC, Yeoreum Yun wrote:

quoted

Hi Brendan,

quoted

On Mon Dec 15, 2025 at 10:06 AM UTC, Yeoreum Yun wrote:
[snip]

quoted

quoted

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.

quoted

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.

But this is what I mean, doesn't this sound like the GFP_ATOMIC comment
I quoted is wrong (or at least, it implies things which are wrong)? The
comment refers specifically to raw_spin_lock() and "strict
non-preemptive contexts". Which sounds like it is being written with
PREEMPT_RT in mind. But that doesn't really match what you've said.

No. I think the comment of GFP_ATOMIC is right.
It definitely said:
  The current implementation *doesn't support* NMI and few other strict
  *non-preemptive contexts (e.g. raw_spin_lock)*.

But this phrasing sounds like there are other non-preemptive contexts
that it _does_ support. I would definitely read this as implying that
plain old preempt_disable() is OK. I don't understand what those "few
other strict contexts" are, nor why the stop_machine() context is
included in them.