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

From: Dmitry Ilvokhin

Date: Wed Apr 15 2026 - 13:43:36 EST


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