Re: [RFC] making nested spin_trylock() work on UP?
From: Harry Yoo (Oracle)
Date: Thu Apr 16 2026 - 23:59:54 EST
On Thu, Apr 16, 2026 at 07:41:02PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 16, 2026 at 7:34 PM Harry Yoo (Oracle) <harry@xxxxxxxxxx> wrote:
> >
> > On Thu, Apr 16, 2026 at 07:37:49AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Apr 16, 2026 at 7:35 AM Harry Yoo (Oracle) <harry@xxxxxxxxxx> wrote:
> > > >
> > > > On Thu, Apr 16, 2026 at 07:26:36AM -0700, Alexei Starovoitov wrote:
> > > > > On Thu Apr 16, 2026 at 3:05 AM PDT, Vlastimil Babka (SUSE) wrote:
> > > > > >> I think we need a special spinlock type that wraps something like this
> > > > > >> and use them when spinlocks can be trylock'd in an unknown context:
> > > > > >> pcp lock, zone lock, per-node partial slab list lock,
> > > > > >> per-node barn lock, etc.
> > > > > >
> > > > > > Soudns like a lot of hassle for a niche config (SMP=n) where nobody would
> > > > > > use e.g. bpf tracing anyway. We already have this in kmalloc_nolock():
> > > > > >
> > > > > > /*
> > > > > > * See the comment for the same check in
> > > > > > * alloc_frozen_pages_nolock_noprof()
> > > > > > */
> > > > > > if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
> > > > > > return NULL;
> > > > > >
> > > > > > It would be trivial to extend this to !SMP. However it wouldn't cover the
> > > > > > kprobe context. Any idea Alexei?
> > > >
> > > > I think Vlastimil meant it'd be trivial to do:
> > > >
> > > > if ((IS_ENABLED(CONFIG_PREEMPT_RT) || !IS_ENABLED(CONFIG_SMP))
> > > > && (in_nmi() || in_hardirq()))
> > > > return NULL;
Just realized that it's unnecessarily restrictive to disallow
hardirq context on UP.
Tried below and it resolves the issue.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2d4b6f1a554e..777499f814f6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7798,6 +7798,11 @@ struct page *alloc_frozen_pages_nolock_noprof(gfp_t gfp_flags, int nid, unsigned
*/
if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
return NULL;
+
+ /* On UP, spin_trylock() always succeeds even when it is locked */
+ if (!IS_ENABLED(CONFIG_SMP) && in_nmi())
+ return NULL;
+
if (!pcp_allowed_order(order))
return NULL;
diff --git a/mm/slub.c b/mm/slub.c
index 2b2d33cc735c..522a0a0d7bcf 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5304,6 +5304,9 @@ void *kmalloc_nolock_noprof(size_t size, gfp_t gfp_flags, int node)
if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
return NULL;
+ if (!IS_ENABLED(CONFIG_SMP) && in_nmi())
+ return NULL;
+
retry:
if (unlikely(size > KMALLOC_MAX_CACHE_SIZE))
return NULL;
> >
> > Thanks for clarifying. You mean not covering the kprobe context is fine.
> >
> > But I have to ask; how is that fine? Wouldn't this leave a small
> > possibility for a kmalloc_nolock() caller to trigger
> > e.g.) use-after-free bug even without noticing? (yeah, very unlikely
> > for somebody to trigger in practice, but not impossible)
> >
> > If it's unlikely to use bpf tracing on UP anyway, it'd be safer to just
> > disallow that to happen to begin with.
>
> Don't fix what is not broken :)
Ack.
> I'm sure there are millions of other issues with UP,
> so there is little to zero chance that anyone can repro such a scenario.
*wishes people running UP tests their kernel with DEBUG_SPINLOCK*
--
Cheers,
Harry / Hyeonggon