Re: [PATCH v4 5/5] locking: Add contended_release tracepoint to spinning locks

[Dmitry Ilvokhin <d@ilvokhin.com>]

On Tue, Apr 14, 2026 at 04:20:26PM -0700, Paul E. McKenney wrote:

[...]

> > +static inline void queued_read_unlock(struct qrwlock *lock)
> > +{
> > +	/*
> > +	 * Trace and unlock are combined in the traced unlock variant so
> > +	 * the compiler does not need to preserve the lock pointer across
> > +	 * the function call, avoiding callee-saved register save/restore
> > +	 * on the hot path.
> > +	 */
> > +	if (tracepoint_enabled(contended_release)) {
> > +		queued_read_unlock_traced(lock);
> > +		return;
> > +	}
> > +
> > +	__queued_read_unlock(lock);
> > +}
> 
> Shouldn't this refactoring be its own separate patch, similar to 4/5?
> 
> That would probably clean up this diff a bit.
> 
> > +
> > +static __always_inline void __queued_write_unlock(struct qrwlock *lock)
> >  {
> >  	smp_store_release(&lock->wlocked, 0);
> >  }
> >  
> >  /**
> > - * queued_rwlock_is_contended - check if the lock is contended
> > + * queued_write_unlock - release write lock of a queued rwlock
> >   * @lock : Pointer to queued rwlock structure
> > - * Return: 1 if lock contended, 0 otherwise
> >   */
> > -static inline int queued_rwlock_is_contended(struct qrwlock *lock)
> > +static inline void queued_write_unlock(struct qrwlock *lock)
> >  {
> > -	return arch_spin_is_locked(&lock->wait_lock);
> > +	/* See comment in queued_read_unlock(). */
> > +	if (tracepoint_enabled(contended_release)) {
> > +		queued_write_unlock_traced(lock);
> > +		return;
> > +	}
> > +
> > +	__queued_write_unlock(lock);
> 
> And the same here, so one patch for interposing __queued_read_unlock()
> and another for interposing __queued_write_unlock().
> 
>

[...]

> And is it possible to have one patch for qspinlock and another for qrwlock?
> It *looks* like it should be.
> 
> 							Thanx, Paul
> 

Thanks for the suggestion, Paul.

I think separate commits for the read and write paths of qrwlock is a
bit too fine-grained, but I like the point about mixing refactoring with
instrumentation and keeping different lock types separate.

I'll split this commit into four.

    locking: Factor out __queued_read_unlock()/__queued_write_unlock()                                                    
    locking: Add contended_release tracepoint to qrwlock
    locking: Factor out queued_spin_release()                                                                             
    locking: Add contended_release tracepoint to qspinlock