linux-mm.kvack.org archive mirror
 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: 6+ 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) [this message]
  -- 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox