Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch 62/63] locking/rtmutex: Add adaptive spinwait mechanism | Date | Wed, 04 Aug 2021 19:49:56 +0200 |
| |
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@goodmis.org> >> >> 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
| |