On 04/05/2017 11:47, Paolo Bonzini wrote: > > > On 04/05/2017 09:39, Fam Zheng wrote: >> On Thu, 04/20 14:00, Paolo Bonzini wrote: >>> + if (atomic_cmpxchg(&mutex->locked, waiters, waiters + 1) != waiters) { >> >> Is it still useful to try the fast path again if there are now even more >> waiters, i.e. "atomic_cmpxchg(...) > waiters"? > > Probably not.
... but when this happens, we don't enter the fast path loop: retry_fast_path: waiters = atomic_read(&mutex->locked); if (waiters == 0) { /* Provide same memory ordering semantics as mutex lock/unlock. */ smp_mb_acquire(); smp_mb_release(); return; } i = 0; while (waiters == 1 && ++i < 1000) { if (atomic_read(&mutex->ctx) == ctx) { break; } waiters = atomic_read(&mutex->locked); if (waiters == 0) { smp_mb_acquire(); smp_mb_release(); return; } cpu_relax(); } if (atomic_cmpxchg(&mutex->locked, waiters, waiters + 1) != waiters) { goto retry_fast_path; } qemu_co_mutex_lock_slowpath(ctx, mutex); qemu_co_mutex_unlock(mutex); The "if (waiters == 0)" fails, and the "while (waiters == 1 && ...)" won't happen either if atomic_cmpxchg returns > 1. So really what you get is a retry of the atomic_cmpxchg. We should introduce atomic_try_cmpxchg. Linux added it recently, too, and it simplifies the code because you don't need to redo the atomic_read. Like this: waiters = atomic_read(&mutex->locked); retry_fast_path: if (waiters == 0) { ... } i = 0; while (waiters == 1 && ++i < 1000) { ... } if (!atomic_cmpxchg(&mutex->locked, &waiters, waiters + 1)) { goto retry_fast_path; } qemu_co_mutex_lock_slowpath(ctx, mutex); qemu_co_mutex_unlock(mutex); Paolo > Paolo > >>> + goto retry_fast_path; >>> + } >>> + >>> + qemu_co_mutex_lock_slowpath(ctx, mutex); >>> + qemu_co_mutex_unlock(mutex); >>> +} >>> + >>> void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex) >>> { >>> Coroutine *self = qemu_coroutine_self(); >>> -- >>> 2.9.3 >>> >>> >>> > >