Re: [PATCH v4 4/4] slub: apply new pw_queue_on() interface

[Sebastian Andrzej Siewior]

On 2026-07-12 19:35:28 [-0300], Leonardo Bras wrote:
> On Wed, May 20, 2026 at 04:53:08PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2026-05-18 22:27:50 [-0300], Leonardo Bras wrote:
> > > @@ -4733,121 +4735,121 @@ void *alloc_from_pcs(struct kmem_cache *s, gfp_t gfp, int node)
> > >  
> > >  	/*
> > >  	 * We assume the percpu sheaves contain only local objects although it's
> > >  	 * not completely guaranteed, so we verify later.
> > >  	 */
> > >  	if (unlikely(node_requested && node != numa_mem_id())) {
> > >  		stat(s, ALLOC_NODE_MISMATCH);
> > >  		return NULL;
> > >  	}
> > >  
> > > -	if (!local_trylock(&s->cpu_sheaves->lock))
> > > +	if (!pw_trylock_local(&s->cpu_sheaves->lock))
> > >  		return NULL;
> > 
> > alloc_from_pcs() can be called from kmalloc_nolock()/ NMI context.
> > I don't remember why exactly local_trylock_t was introduced here instead
> > of a per-CPU spinlock_t. 
> 
> Probably to save the cost of using atomic operations on locking, and having 
> about the same restrictions that would allow using local_locks
> 
> > But there should be nothing wrong with a
> > trylock on it from NMI as you do here.
> 
> Awesome!

The problem is always the unlock which requires full locking and is
usually the problem from NMI.

> > 
> > One thing worth noting, on !PREEMPT_RT, spin_trylock() always succeeds
> > on UP. kmalloc_nolock() checks for it, not sure about other callers.
> 
> 
> Sorry, I did not sure I understand that part. 
> You mean we have since it always returns true, we may be in NMI context, 
> after it was interrupted holding this lock, and it will return true which 
> will use the protected area even though the lock should avoid it?

from include/linux/spinlock_api_up.h:
| static __always_inline int _raw_spin_trylock(raw_spinlock_t *lock)
|         __cond_acquires(true, lock)
| {
|         __LOCK(lock);
|         return 1;
| }

on UP a spin_trylock() always succeeds.

> Humm, but if that scenario exist, then is it actually ok to return true on 
> trylock() in that scenario?
> 
> Thanks!
> Leo

Sebastian