linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Harry Yoo (Oracle)" <harry@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: "Vlastimil Babka (SUSE)" <vbabka@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Waiman Long <longman@redhat.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Steven Rostedt <rostedt@goodmis.org>, Hao Li <hao.li@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	Brendan Jackman <jackmanb@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>, Zi Yan <ziy@nvidia.com>,
	Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	Roman Gushchin <roman.gushchin@linux.dev>
Subject: Re: [RFC] making nested spin_trylock() work on UP?
Date: Fri, 17 Apr 2026 12:59:47 +0900	[thread overview]
Message-ID: <aeGwM6FPL-yJEs1q@hyeyoo> (raw)
In-Reply-To: <CAADnVQKLNDitD+WfNCr-nB955GVO7YZeHMjkuOs-=sqnvYyTxw@mail.gmail.com>

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@kernel.org> 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@kernel.org> 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


  reply	other threads:[~2026-04-17  3:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-13 11:57 Vlastimil Babka
2026-02-14  6:28 ` Matthew Wilcox
2026-02-14 16:32   ` Linus Torvalds
2026-02-16 10:32     ` Vlastimil Babka
2026-04-15 18:44   ` Harry Yoo (Oracle)
2026-04-16 10:05     ` Vlastimil Babka (SUSE)
2026-04-16 14:26       ` Alexei Starovoitov
2026-04-16 14:35         ` Harry Yoo (Oracle)
2026-04-16 14:37           ` Alexei Starovoitov
2026-04-17  2:34             ` Harry Yoo (Oracle)
2026-04-17  2:41               ` Alexei Starovoitov
2026-04-17  3:59                 ` Harry Yoo (Oracle) [this message]
2026-04-17  5:37                   ` Alexei Starovoitov
  -- strict thread matches above, loose matches on Subject: below --
2026-02-13 11:57 Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aeGwM6FPL-yJEs1q@hyeyoo \
    --to=harry@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bigeasy@linutronix.de \
    --cc=cl@gentwo.org \
    --cc=hannes@cmpxchg.org \
    --cc=hao.li@linux.dev \
    --cc=jackmanb@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=surenb@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@kernel.org \
    --cc=vbabka@suse.cz \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox