[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aUE8bwUVa6jSUft1@e129823.arm.com>
Date: Tue, 16 Dec 2025 11:03:11 +0000
From: Yeoreum Yun <yeoreum.yun@....com>
To: Brendan Jackman <jackmanb@...gle.com>
Cc: akpm@...ux-foundation.org, david@...nel.org, lorenzo.stoakes@...cle.com,
Liam.Howlett@...cle.com, vbabka@...e.cz, rppt@...nel.org,
surenb@...gle.com, mhocko@...e.com, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, hannes@...xchg.org,
ziy@...dia.com, bigeasy@...utronix.de, clrkwllms@...nel.org,
rostedt@...dmis.org, catalin.marinas@....com, will@...nel.org,
ryan.roberts@....com, kevin.brodsky@....com, dev.jain@....com,
yang@...amperecomputing.com, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
linux-rt-devel@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] arm64: mmu: use pagetable_alloc_nolock() while
stop_machine()
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
Powered by blists - more mailing lists