Re: [patch 62/63] locking/rtmutex: Add adaptive spinwait mechanism

[Thomas Gleixner]

On Wed, Aug 04 2021 at 14:30, Peter Zijlstra wrote:
> On Fri, Jul 30, 2021 at 03:51:09PM +0200, Thomas Gleixner wrote:
>> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
>> 
>> Going to sleep when a spinlock or rwlock is contended can be quite
>> inefficient when the contention time is short and the lock owner is running
>> on a different CPU. The MCS mechanism is not applicable to rtmutex based
>> locks, so provide a simple adaptive spinwait mechanism for the RT specific
>> spin/rwlock implementations.
>
> A better Changelog would explain *why* OSQ does not apply. I'm thinking
> this ie because the (spin) waiters can be of different priorities and we
> need to ensure the highest prio waiter gets to win?
>
> AFAICT that isn't true even without OSQ, you just get a thundering herd
> and the higher prio waiter has a better chance of winning the race but
> all bets are off either way around.

Will do.

>> +#ifdef CONFIG_SMP
>
> Existing convention would make that:
>
> #ifdef CONFIG_RTMUTEX_SPIN_ON_OWNER
>
> But I suppose that's indeed not required if we don't use OSQ.

Right.

>> +/*
>> + * Note that owner is a speculative pointer and dereferencing relies
>> + * on rcu_read_lock() and the check against the lock owner.
>> + */
>> +static bool rtlock_adaptive_spinwait(struct rt_mutex_base *lock,
>> +				     struct task_struct *owner)
>
> similarly, this would be:
>
>   rt_mutex_spin_on_owner()

Duh.
>
> Esp. when this will be on rtmutex unconditionally, you want to mirror
> the full set of conditions we also have on mutex_spin_on_owner():
>
> 	|| need_resched() || vcpu_is_preempted(task_cpu(owner))

Sure.

>> +			res = false;
>> +			break;
>> +		}
>> +		cpu_relax();
>> +	}
>> +	rcu_read_unlock();
>> +	return res;
>> +}
>
> Additionally, we could consider adding something that would compare the
> current prio to the top_waiter prio and terminate the loop if we're
> found to be of lower prio, but lifetime issues are probably going to
> make that 'interesting'.

It's only the top priority waiter which can spin. If all of them start
spinning then everything goes down the drain.

Thanks,

        tglx