Re: [PATCH v3 5/6] slab: Introduce kmalloc_nolock() and kfree_nolock().

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Thu, Jul 17, 2025 at 2:18 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> >
> > When "bool allow_spin" was there in Sebastian's version it definitely
> > looked cleaner as a proper function,
> > but now, if (!IS_ENABLED(CONFIG_PREEMPT_RT)) can be
> > #ifdef CONFIG_PREEMPT_RT
> > and the comment will look normal (without ugly backslashes)
> > So yeah. I'll convert it to macro.
>
> To clarify, the ideal I think would be e.g.
>
> #if defined(CONFIG_PREEMPT_RT) || !defined(CONFIG_LOCKDEP)
>
> local_lock_irqsave();
>
> #else
>
> lockdep_assert(local_trylock_irqsave());
>
> #endif
>
> This should mean that without lockdep we just trust the code to be correct
> (kmalloc_nolock() using local_lock_held() properly before calling here, and
> kmalloc() callers not being in an unsupported context, as before this
> series) with no checking on both RT and !RT.
>
> With lockdep, on RT lockdep does its checking in local_lock_irqsave()
> normally. On !RT we need to use trylock to avoid false positives in nmi, but
> lockdep_assert() will catch a bug still in case of a true positive.
>
> At least I hope I got it right...

Yes. Exactly what I had in mind.

> > My preference is to add a comment saying that only objects
> > allocated by kmalloc_nolock() should be freed by kfree_nolock().
>
> We could go with that until someone has a case for changing this, and then
> handle kmemleak and kfence with defer_free()...

Good. Will go with warning/comment for now.

At least from bpf infra pov the context where free-ing is done
is better controlled than allocation.
Free is often in call_rcu() or call_rcu_tasks_trace() callback.
Whereas the context of kmalloc_nolock() is impossible to predict
when it comes to tracing bpf progs.
So the issues from kmalloc_nolock() -> kfree() transition are more
likely to occur, but bpf progs won't be using these primitives
directly, of course. The existing layers of protection
like bpf_obj_new()/bpf_obj_drop() will continue to be the interface.

Fun fact... initially I was planning to implement kmalloc_nolock()
only :) since bpf use case of freeing is after rcu and rcu_tasks_trace GP,
but once I realized that slab is already recursive and kmalloc_nolock()
has to be able to free deep inside, I figured I had to implement
kfree_nolock() in the same patch.
I'm talking about
alloc_slab_obj_exts() -> vec = kmalloc_nolock() -> kfree_nolock(vec) path.