From: Jeremy Fitzhardinge on
On 06/30/2010 04:03 PM, Jan Beulich wrote:
>>>> On 30.06.10 at 15:23, Jeremy Fitzhardinge <jeremy(a)goop.org> wrote:
>>>>
>> For spinlocks, the pvop calls should only be in the slow case: when a
>> spinlock has been spinning for long enough, and on unlock when there's
>> someone waiting for the lock. The fastpath (no contention lock and
>> unlock) should have no extra calls.
>>
> Then what was all that performance regression noise concerning
> pvops spinlocks about, leading to CONFIG_PARAVIRT_SPINLOCKS
> being separated from the base CONFIG_PARAVIRT?
>

Nobody knows. The pv spinlocks appeared to cause a 5% performance
regression on some benchmarks, which is wildly huge. It appears to
trigger some kind of microarchitectural catastrophe on some Intel cpus,
perhaps relating to the extra call in the path or something.

> Afaics the unlock still involves a function call *in all cases* with
> pvops spinlocks, whereas it's a single inline instruction without.
>

No. The unlock path can see if there are any further waiters by looking
at the ticket in the, and only do the kick call if there are some.

J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeremy Fitzhardinge on
On 06/30/2010 04:36 PM, Jan Beulich wrote:
> Are we perhaps talking about different things? I'm referring to
>
> static __always_inline void arch_spin_unlock(struct arch_spinlock *lock)
> {
> PVOP_VCALL1(pv_lock_ops.spin_unlock, lock);
> }
>
> which is an indirect call which, as I understand it, gets replaced
> with a direct one at runtime. But it remains to be a call (as opposed
> to being a single inc instructions without CONFIG_PARAVIRT_SPINLOCKS).
>

Sorry, I'm referring to pv ticketlocks, not the current
PARAVIRT_SPINLOCKS code. I agree the current PARAVIRT_SPINLOCKS
implementation is suboptimal and needs to be replaced with something
that's only called on the slow path. I just think the existing
paravirt_ops mechanism can be used to implement it rather than adding
something new.

J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/