On 06/12/2014 04:17 AM, Peter Zijlstra wrote:
On Fri, May 30, 2014 at 11:44:00AM -0400, Waiman Long wrote:@@ -19,13 +19,46 @@ extern struct static_key virt_unfairlocks_enabled; * that the clearing the lock bit is done ASAP without artificial delay * due to compiler optimization. */ +#ifdef CONFIG_PARAVIRT_SPINLOCKS +static __always_inline void __queue_spin_unlock(struct qspinlock *lock) +#else static inline void queue_spin_unlock(struct qspinlock *lock) +#endif { barrier(); ACCESS_ONCE(*(u8 *)lock) = 0; barrier(); } +#ifdef CONFIG_PARAVIRT_SPINLOCKS +/* + * The lock byte can have a value of _Q_LOCKED_SLOWPATH to indicate + * that it needs to go through the slowpath to do the unlocking. + */ +#define _Q_LOCKED_SLOWPATH (_Q_LOCKED_VAL | 2) + +extern void queue_spin_unlock_slowpath(struct qspinlock *lock); + +static inline void queue_spin_unlock(struct qspinlock *lock) +{ + barrier(); + if (static_key_false(¶virt_spinlocks_enabled)) { + /* + * Need to atomically clear the lock byte to avoid racing with + * queue head waiter trying to set _QLOCK_LOCKED_SLOWPATH. + */ + if (likely(cmpxchg((u8 *)lock, _Q_LOCKED_VAL, 0) + == _Q_LOCKED_VAL)) + return; + else + queue_spin_unlock_slowpath(lock); + + } else { + __queue_spin_unlock(lock); + } + barrier(); +} +#endif /* CONFIG_PARAVIRT_SPINLOCKS */Ideally we'd make all this use alternatives or so, such that the actual function remains short enough to actually inline; static inline void queue_spin_unlock(struct qspinlock *lock) { pv_spinlock_alternative( ACCESS_ONCE(*(u8 *)lock) = 0, pv_queue_spin_unlock(lock)); } Or however that trickery works.
I think the paravirt version of the unlock function is already short enough. In addition, whenever PARAVIRT_SPINLOCKS is enabled, the inlining of the unlock function is disabled so that the jump label paravirt_spinlocks_enabled won't be used everywhere in the core kernel and the kernel modules. This is true for the ticket spinlock and will also be true for the queue spinlock.
I don't have a good understanding of the kernel alternatives mechanism. I think it allow boot time modification of the kernel code according to the CPU type, for example. The jump label mechanism is similar and is much easier to use than the alternatives. I don't see a need to use the alternatives unless you saw a big advantage of using it which I am not aware of.
-Longman -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
