All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Harry Yoo (Oracle)" <harry@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: 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>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	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: Thu, 16 Apr 2026 03:44:57 +0900	[thread overview]
Message-ID: <ad_cqe51pvr1WaDg@hyeyoo> (raw)
In-Reply-To: <aZAWGwZP_Z75YHKt@casper.infradead.org>

[+Cc Alexei for _nolock() APIs]
[+Cc SLAB ALLOCATOR and PAGE ALLOCATOR folks]

I was testing kmalloc_nolock() on UP and I think
I'm dealt with a similar issue...

On Sat, Feb 14, 2026 at 06:28:43AM +0000, Matthew Wilcox wrote:
> On Fri, Feb 13, 2026 at 12:57:43PM +0100, Vlastimil Babka wrote:
> > The page allocator has been using a locking scheme for its percpu page
> > caches (pcp) for years now, based on spin_trylock() with no _irqsave() part.
> > The point is that if we interrupt the locked section, we fail the trylock
> > and just fallback to something that's more expensive, but it's rare so we
> > don't need to pay the irqsave cost all the time in the fastpaths.
> > 
> > It's similar to but not exactly local_trylock_t (which is also newer anyway)
> > because in some cases we do lock the pcp of a non-local cpu to flush it, in
> > a way that's cheaper than IPI or queue_work_on().
> > 
> > The complication of this scheme has been UP non-debug spinlock
> > implementation which assumes spin_trylock() can't fail on UP and has no
> > state to track it. It just doesn't anticipate this usage scenario.

This is not the only scenario that doesn't work.

I was testing "calling {kmalloc,kfree}_nolock() in an NMI handler
when the CPU is calling kmalloc() & kfree()" [1] scenario.

Weirdly it's broken (dmesg at the end of the email) on UP since v6.18,
where {kmalloc,kfree}_nolock() APIs were introduced.

[1] https://lore.kernel.org/linux-mm/20260406090907.11710-3-harry@kernel.org

> > So to
> > work around that we disable IRQs on UP, complicating the implementation.
> > Also recently we found years old bug in the implementation - see
> > 038a102535eb ("mm/page_alloc: prevent pcp corruption with SMP=n").

In the case mentioned above, disabling IRQs doesn't work as the handler
can be called in an NMI context.

{kmalloc,kfree}_nolock()->spin_trylock_irqsave() can succeed on UP
when the CPU already acquired the spinlock w/ IRQs disabled.

> > So my question is if we could have spinlock implementation supporting this
> > nested spin_trylock() usage, or if the UP optimization is still considered
> > too important to lose it. I was thinking:
> > 
> > - remove the UP implementation completely - would it increase the overhead
> > on SMP=n systems too much and do we still care?
> > 
> > - make the non-debug implementation a bit like the debug one so we do have
> > the 'locked' state (see include/linux/spinlock_up.h and lock->slock). This
> > also adds some overhead but not as much as the full SMP implementation?
> 
> What if we use an atomic_t on UP to simulate there being a spinlock,
> but only for pcp?  Your demo shows pcp_spin_trylock() continuing to
> exist, so how about doing something like:
> 
> #ifdef CONFIG_SMP
> #define pcp_spin_trylock(ptr)						\
> ({									\
> 	struct per_cpu_pages *__ret;					\
> 	__ret = pcpu_spin_trylock(struct per_cpu_pages, lock, ptr);	\
> 	__ret;								\
> })
> #else
> static atomic_t pcp_UP_lock = ATOMIC_INIT(0);
> #define pcp_spin_trylock(ptr)						\
> ({									\
> 	struct per_cpu_pages *__ret = NULL;				\
> 	if (atomic_try_cmpxchg(&pcp_UP_lock, 0, 1))			\
> 		__ret = (void *)&pcp_UP_lock;				\
> 	__ret;								\
> });
> #endif
>
> (obviously you need pcp_spin_lock/pcp_spin_unlock also defined)
> 
> That only costs us 4 extra bytes on UP, rather than 4 bytes per spinlock.
> And some people still use routers with tiny amounts of memory and a
> single CPU, or retrocomputers with single CPUs.

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.

dmesg here, HEAD is a commit that adds the test case, on top of
commit af92793e52c3a ("slab: Introduce kmalloc_nolock() and
kfree_nolock()."):
> 
> [    3.658916] ------------[ cut here ]------------
> [    3.659492] perf: interrupt took too long (5015 > 5005), lowering kernel.perf_event_max_sample_rate to 39000
> [    3.660800] kernel BUG at mm/slub.c:4382!

This is BUG_ON(new.frozen) in freeze_slab(), which implies that
somebody else has taken it off list and froze it already (which should
have been prevented by the spinlock)

> [    3.661674] Oops: invalid opcode: 0000 [#1] NOPTI
> [    3.662427] CPU: 0 UID: 0 PID: 256 Comm: kunit_try_catch Tainted: G            E    N  6.17.0-rc3+ #24 PREEMPTLAZY
> [    3.663270] Tainted: [E]=UNSIGNED_MODULE, [N]=TEST
> [    3.663658] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [    3.664571] RIP: 0010:___slab_alloc (mm/slub.c:4382 (discriminator 1) mm/slub.c:4599 (discriminator 1)) 
> [ 3.664949] Code: 4c 24 78 e8 32 cc ff ff 84 c0 0f 85 09 fa ff ff 49 8b 4c 24 28 4d 8b 6c 24 20 48 89 c8 48 89 4c 24 78 48 c1 e8 18 84 c0 79 b3 <0f> 0b 41 8b 46 10 a9 87 04 00 00 74 a1 a8 80 75 24 49 89 dd e9 09

-- 
Cheers,
Harry / Hyeonggon

> All code
> ========
>    0:	4c 24 78             	rex.WR and $0x78,%al
>    3:	e8 32 cc ff ff       	call   0xffffffffffffcc3a
>    8:	84 c0                	test   %al,%al
>    a:	0f 85 09 fa ff ff    	jne    0xfffffffffffffa19
>   10:	49 8b 4c 24 28       	mov    0x28(%r12),%rcx
>   15:	4d 8b 6c 24 20       	mov    0x20(%r12),%r13
>   1a:	48 89 c8             	mov    %rcx,%rax
>   1d:	48 89 4c 24 78       	mov    %rcx,0x78(%rsp)
>   22:	48 c1 e8 18          	shr    $0x18,%rax
>   26:	84 c0                	test   %al,%al
>   28:	79 b3                	jns    0xffffffffffffffdd
>   2a:*	0f 0b                	ud2		<-- trapping instruction
>   2c:	41 8b 46 10          	mov    0x10(%r14),%eax
>   30:	a9 87 04 00 00       	test   $0x487,%eax
>   35:	74 a1                	je     0xffffffffffffffd8
>   37:	a8 80                	test   $0x80,%al
>   39:	75 24                	jne    0x5f
>   3b:	49 89 dd             	mov    %rbx,%r13
>   3e:	e9                   	.byte 0xe9
>   3f:	09                   	.byte 0x9
> 
> Code starting with the faulting instruction
> ===========================================
>    0:	0f 0b                	ud2
>    2:	41 8b 46 10          	mov    0x10(%r14),%eax
>    6:	a9 87 04 00 00       	test   $0x487,%eax
>    b:	74 a1                	je     0xffffffffffffffae
>    d:	a8 80                	test   $0x80,%al
>    f:	75 24                	jne    0x35
>   11:	49 89 dd             	mov    %rbx,%r13
>   14:	e9                   	.byte 0xe9
>   15:	09                   	.byte 0x9
> [    3.666437] RSP: 0018:ffffc9d4001d3c80 EFLAGS: 00010282
> [    3.666865] RAX: 0000000000000080 RBX: ffff8990fffd2e20 RCX: 0000000080400040
> [    3.667440] RDX: ffff8990c0051c48 RSI: 0000000000400cc0 RDI: ffff8990c0054100
> [    3.668018] RBP: ffffc9d4001d3d40 R08: 0000000000400cc0 R09: ffff8990c0051c40
> [    3.668628] R10: ffff8990fffd2e20 R11: ffff8990fffd2e20 R12: ffffec0e04031cc0
> [    3.669222] R13: 0000000000000000 R14: ffff8990c0054100 R15: ffffffffc04e8174
> [    3.669815] FS:  0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
> [    3.670475] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    3.670960] CR2: 00007ffcf4c95a68 CR3: 000000001f052000 CR4: 0000000000750ef0
> [    3.671554] PKRU: 55555554
> [    3.671799] Call Trace:
> [    3.672012]  <TASK>
> [    3.672199] ? test_kmalloc_kfree_nolock (lib/tests/slub_kunit.c:357 (discriminator 4)) slub_kunit 
> [    3.672704] ? test_kmalloc_kfree_nolock (lib/tests/slub_kunit.c:357 (discriminator 4)) slub_kunit 
> [    3.673211] __kmalloc_cache_noprof (mm/slub.c:4722 mm/slub.c:4798 mm/slub.c:5209 mm/slub.c:5695) 
> [    3.673595] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:183) 
> [    3.674003] test_kmalloc_kfree_nolock (lib/tests/slub_kunit.c:357 (discriminator 4)) slub_kunit 
> [    3.674475] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:183) 
> [    3.674869] ? test_kmalloc_kfree_nolock (lib/tests/slub_kunit.c:357 (discriminator 4)) slub_kunit 
> [    3.675354] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:183) 
> [    3.675754] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:183) 
> [    3.676144] ? srso_alias_return_thunk (arch/x86/lib/retpoline.S:183) 
> [    3.676535] ? __switch_to (./arch/x86/include/asm/cpufeature.h:101 arch/x86/kernel/process_64.c:378 arch/x86/kernel/process_64.c:666) 
> [    3.676848] ? __pfx_kunit_generic_run_threadfn_adapter (lib/kunit/try-catch.c:26) kunit 
> [    3.677395] kunit_try_run_case (lib/kunit/test.c:441 lib/kunit/test.c:484) kunit 
> [    3.677802] ? __pfx_kunit_generic_run_threadfn_adapter (lib/kunit/try-catch.c:26) kunit 
> [    3.678355] kunit_generic_run_threadfn_adapter (lib/kunit/try-catch.c:31) kunit 
> [    3.678857] kthread (kernel/kthread.c:463) 
> [    3.679130] ? __pfx_kthread (kernel/kthread.c:412) 
> [    3.679442] ret_from_fork (arch/x86/kernel/process.c:154) 
> [    3.679759] ? __pfx_kthread (kernel/kthread.c:412) 
> [    3.680071] ret_from_fork_asm (arch/x86/entry/entry_64.S:255) 
> [    3.680397]  </TASK>
> [    3.680585] Modules linked in: slub_kunit(E) kunit(E) intel_rapl_msr(E) intel_rapl_common(E) aesni_intel(E) ghash_clmulni_intel(E) kvm_amd(E) ccp(E) kvm(E) irqbypass(E) input_leds(E) i2c_piix4(E) i2c_smbus(E) mac_hid(E)
> [    3.682187] ---[ end trace 0000000000000000 ]---
> [    3.683108] RIP: 0010:___slab_alloc (mm/slub.c:4382 (discriminator 1) mm/slub.c:4599 (discriminator 1)) 
> [ 3.684032] Code: 4c 24 78 e8 32 cc ff ff 84 c0 0f 85 09 fa ff ff 49 8b 4c 24 28 4d 8b 6c 24 20 48 89 c8 48 89 4c 24 78 48 c1 e8 18 84 c0 79 b3 <0f> 0b 41 8b 46 10 a9 87 04 00 00 74 a1 a8 80 75 24 49 89 dd e9 09
> All code
> ========
>    0:	4c 24 78             	rex.WR and $0x78,%al
>    3:	e8 32 cc ff ff       	call   0xffffffffffffcc3a
>    8:	84 c0                	test   %al,%al
>    a:	0f 85 09 fa ff ff    	jne    0xfffffffffffffa19
>   10:	49 8b 4c 24 28       	mov    0x28(%r12),%rcx
>   15:	4d 8b 6c 24 20       	mov    0x20(%r12),%r13
>   1a:	48 89 c8             	mov    %rcx,%rax
>   1d:	48 89 4c 24 78       	mov    %rcx,0x78(%rsp)
>   22:	48 c1 e8 18          	shr    $0x18,%rax
>   26:	84 c0                	test   %al,%al
>   28:	79 b3                	jns    0xffffffffffffffdd
>   2a:*	0f 0b                	ud2		<-- trapping instruction
>   2c:	41 8b 46 10          	mov    0x10(%r14),%eax
>   30:	a9 87 04 00 00       	test   $0x487,%eax
>   35:	74 a1                	je     0xffffffffffffffd8
>   37:	a8 80                	test   $0x80,%al
>   39:	75 24                	jne    0x5f
>   3b:	49 89 dd             	mov    %rbx,%r13
>   3e:	e9                   	.byte 0xe9
>   3f:	09                   	.byte 0x9
> 
> Code starting with the faulting instruction
> ===========================================
>    0:	0f 0b                	ud2
>    2:	41 8b 46 10          	mov    0x10(%r14),%eax
>    6:	a9 87 04 00 00       	test   $0x487,%eax
>    b:	74 a1                	je     0xffffffffffffffae
>    d:	a8 80                	test   $0x80,%al
>    f:	75 24                	jne    0x35
>   11:	49 89 dd             	mov    %rbx,%r13
>   14:	e9                   	.byte 0xe9
>   15:	09                   	.byte 0x9
> [    3.686093] RSP: 0018:ffffc9d4001d3c80 EFLAGS: 00010282
> [    3.687036] RAX: 0000000000000080 RBX: ffff8990fffd2e20 RCX: 0000000080400040
> [    3.688128] RDX: ffff8990c0051c48 RSI: 0000000000400cc0 RDI: ffff8990c0054100
> [    3.689244] RBP: ffffc9d4001d3d40 R08: 0000000000400cc0 R09: ffff8990c0051c40
> [    3.690353] R10: ffff8990fffd2e20 R11: ffff8990fffd2e20 R12: ffffec0e04031cc0
> [    3.691476] R13: 0000000000000000 R14: ffff8990c0054100 R15: ffffffffc04e8174
> [    3.692864] FS:  0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
> [    3.694016] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    3.694997] CR2: 00007ffcf4c95a68 CR3: 000000001f052000 CR4: 0000000000750ef0
> [    3.696109] PKRU: 55555554
> [    3.696834] note: kunit_try_catch[256] exited with preempt_count 1
> [    3.696910] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: vprintk_store (kernel/printk/printk.c:2358) 
> [    3.698650] Kernel Offset: 0x1d000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)


  parent reply	other threads:[~2026-04-15 18:45 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-13 11:57 [RFC] making nested spin_trylock() work on UP? 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) [this message]
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)
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=ad_cqe51pvr1WaDg@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@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.