On Thu, Jul 17, 2025 at 2:18 AM Vlastimil Babka <vbabka@xxxxxxx> 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.
