search for: paravirt_unfairlocks_enabled

...environment is generally not a good idea as there is a possibility of lock starvation for a heavily contended lock. This patch add a new configuration option for the x86 architecture to enable the use of unfair queue spinlock (PARAVIRT_UNFAIR_LOCKS) in a real para-virtualized guest. A jump label (paravirt_unfairlocks_enabled) is used to switch between a fair and an unfair version of the spinlock code. This jump label will only be enabled in a real PV guest. Enabling this configuration feature decreases the performance of an uncontended lock-unlock operation by about 1-2%. Signed-off-by: Waiman Long <Waiman.Long at...

...tive environment is generally not a good idea as there is a possibility of lock starvation for a heavily contended lock. This patch adds a new configuration option for the x86 architecture to enable the use of unfair queue spinlock (PARAVIRT_UNFAIR_LOCKS) in a para-virtualized guest. A jump label (paravirt_unfairlocks_enabled) is used to switch between a fair and an unfair version of the spinlock code. This jump label will only be enabled in a virtual guest where the X86_FEATURE_HYPERVISOR feature bit is set. Enabling this configuration feature causes a slight decrease the performance of an uncontended lock-unlock oper...

...environment is generally not a good idea as there is a possibility of lock starvation for a heavily contended lock. This patch add a new configuration option for the x86 architecture to enable the use of unfair queue spinlock (PARAVIRT_UNFAIR_LOCKS) in a real para-virtualized guest. A jump label (paravirt_unfairlocks_enabled) is used to switch between a fair and an unfair version of the spinlock code. This jump label will only be enabled in a real PV guest. Enabling this configuration feature causes a slight decrease the performance of an uncontended lock-unlock operation by about 1-2% mainly due to the use of a stati...

...rtualization and only then activate itself? And when run under baremetal don't enable? > > This patch add a new configuration option for the x86 > architecture to enable the use of unfair queue spinlock > (PARAVIRT_UNFAIR_LOCKS) in a real para-virtualized guest. A jump label > (paravirt_unfairlocks_enabled) is used to switch between a fair and > an unfair version of the spinlock code. This jump label will only be > enabled in a real PV guest. As opposed to fake PV guest :-) I think you can remove the 'real'. > > Enabling this configuration feature decreases the performance of...

...rtualization and only then activate itself? And when run under baremetal don't enable? > > This patch add a new configuration option for the x86 > architecture to enable the use of unfair queue spinlock > (PARAVIRT_UNFAIR_LOCKS) in a real para-virtualized guest. A jump label > (paravirt_unfairlocks_enabled) is used to switch between a fair and > an unfair version of the spinlock code. This jump label will only be > enabled in a real PV guest. As opposed to fake PV guest :-) I think you can remove the 'real'. > > Enabling this configuration feature decreases the performance of...

On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > +static inline void arch_spin_lock(struct qspinlock *lock) > +{ > + if (static_key_false(&paravirt_unfairlocks_enabled)) > + queue_spin_lock_unfair(lock); > + else > + queue_spin_lock(lock); > +} So I would have expected something like: if (static_key_false(&paravirt_spinlock)) { while (!queue_spin_trylock(lock)) cpu_relax(); return; } At the top of queue_spin_lock_slowpath(). > +s...

On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > +static inline void arch_spin_lock(struct qspinlock *lock) > +{ > + if (static_key_false(&paravirt_unfairlocks_enabled)) > + queue_spin_lock_unfair(lock); > + else > + queue_spin_lock(lock); > +} So I would have expected something like: if (static_key_false(&paravirt_spinlock)) { while (!queue_spin_trylock(lock)) cpu_relax(); return; } At the top of queue_spin_lock_slowpath(). > +s...

On 03/13/2014 11:15 AM, Peter Zijlstra wrote: > On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: >> +static inline void arch_spin_lock(struct qspinlock *lock) >> +{ >> + if (static_key_false(&paravirt_unfairlocks_enabled)) >> + queue_spin_lock_unfair(lock); >> + else >> + queue_spin_lock(lock); >> +} > So I would have expected something like: > > if (static_key_false(&paravirt_spinlock)) { > while (!queue_spin_trylock(lock)) > cpu_relax(); > return; > } &g...

...4 > --- a/kernel/locking/qspinlock.c > +++ b/kernel/locking/qspinlock.c > @@ -227,6 +227,14 @@ static __always_inline int get_qlock(struct qspinlock *lock) > { > struct __qspinlock *l = (void *)lock; > > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > + if (static_key_false(&paravirt_unfairlocks_enabled)) > + /* > + * Need to use atomic operation to get the lock when > + * lock stealing can happen. > + */ > + return cmpxchg(&l->locked, 0, _Q_LOCKED_VAL) == 0; That's missing {}. > +#endif > barrier(); > ACCESS_ONCE(l->locked) = _Q_LOCKED_VAL; &g...

...4 > --- a/kernel/locking/qspinlock.c > +++ b/kernel/locking/qspinlock.c > @@ -227,6 +227,14 @@ static __always_inline int get_qlock(struct qspinlock *lock) > { > struct __qspinlock *l = (void *)lock; > > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > + if (static_key_false(&paravirt_unfairlocks_enabled)) > + /* > + * Need to use atomic operation to get the lock when > + * lock stealing can happen. > + */ > + return cmpxchg(&l->locked, 0, _Q_LOCKED_VAL) == 0; That's missing {}. > +#endif > barrier(); > ACCESS_ONCE(l->locked) = _Q_LOCKED_VAL; &g...

...t; + > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > +/* > + * Enable unfair lock if running in a real para-virtualized environment > + */ > +static __init int kvm_unfair_locks_init_jump(void) > +{ > + if (!kvm_para_available()) > + return 0; > + > + static_key_slow_inc(&paravirt_unfairlocks_enabled); > + printk(KERN_INFO "KVM setup unfair spinlock\n"); > + > + return 0; > +} > +early_initcall(kvm_unfair_locks_init_jump); > +#endif > I think this should apply to all paravirt implementations, unless pv_lock_ops.kick_cpu != NULL. Paolo

...t; + > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > +/* > + * Enable unfair lock if running in a real para-virtualized environment > + */ > +static __init int kvm_unfair_locks_init_jump(void) > +{ > + if (!kvm_para_available()) > + return 0; > + > + static_key_slow_inc(&paravirt_unfairlocks_enabled); > + printk(KERN_INFO "KVM setup unfair spinlock\n"); > + > + return 0; > +} > +early_initcall(kvm_unfair_locks_init_jump); > +#endif > I think this should apply to all paravirt implementations, unless pv_lock_ops.kick_cpu != NULL. Paolo

...s used for this purpose and will be enabled by the appropriate KVM or Xen code. >> This patch add a new configuration option for the x86 >> architecture to enable the use of unfair queue spinlock >> (PARAVIRT_UNFAIR_LOCKS) in a real para-virtualized guest. A jump label >> (paravirt_unfairlocks_enabled) is used to switch between a fair and >> an unfair version of the spinlock code. This jump label will only be >> enabled in a real PV guest. > As opposed to fake PV guest :-) I think you can remove the 'real'. Yes, you are right. I will remove that in the next series. >...

...t; + > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > +/* > + * Enable unfair lock if running in a real para-virtualized environment > + */ > +static __init int kvm_unfair_locks_init_jump(void) > +{ > + if (!kvm_para_available()) > + return 0; > + > + static_key_slow_inc(&paravirt_unfairlocks_enabled); > + printk(KERN_INFO "KVM setup unfair spinlock\n"); > + > + return 0; > +} > +early_initcall(kvm_unfair_locks_init_jump); > +#endif > No! Please do what I asked you to do. You are not handling Hyper-V or VMWare. Just use X86_FEATURE_HYPERVISOR and it will cove...

...t; + > +#ifdef CONFIG_PARAVIRT_UNFAIR_LOCKS > +/* > + * Enable unfair lock if running in a real para-virtualized environment > + */ > +static __init int kvm_unfair_locks_init_jump(void) > +{ > + if (!kvm_para_available()) > + return 0; > + > + static_key_slow_inc(&paravirt_unfairlocks_enabled); > + printk(KERN_INFO "KVM setup unfair spinlock\n"); > + > + return 0; > +} > +early_initcall(kvm_unfair_locks_init_jump); > +#endif > No! Please do what I asked you to do. You are not handling Hyper-V or VMWare. Just use X86_FEATURE_HYPERVISOR and it will cove...

...13, 2014 at 04:05:19PM -0400, Waiman Long wrote: > On 03/13/2014 11:15 AM, Peter Zijlstra wrote: > >On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > >>+static inline void arch_spin_lock(struct qspinlock *lock) > >>+{ > >>+ if (static_key_false(&paravirt_unfairlocks_enabled)) > >>+ queue_spin_lock_unfair(lock); > >>+ else > >>+ queue_spin_lock(lock); > >>+} > >So I would have expected something like: > > > > if (static_key_false(&paravirt_spinlock)) { > > while (!queue_spin_trylock(lock)) > > c...

...13, 2014 at 04:05:19PM -0400, Waiman Long wrote: > On 03/13/2014 11:15 AM, Peter Zijlstra wrote: > >On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > >>+static inline void arch_spin_lock(struct qspinlock *lock) > >>+{ > >>+ if (static_key_false(&paravirt_unfairlocks_enabled)) > >>+ queue_spin_lock_unfair(lock); > >>+ else > >>+ queue_spin_lock(lock); > >>+} > >So I would have expected something like: > > > > if (static_key_false(&paravirt_spinlock)) { > > while (!queue_spin_trylock(lock)) > > c...

...ng wrote: > >>On 03/13/2014 11:15 AM, Peter Zijlstra wrote: > >>>On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > >>>>+static inline void arch_spin_lock(struct qspinlock *lock) > >>>>+{ > >>>>+ if (static_key_false(&paravirt_unfairlocks_enabled)) > >>>>+ queue_spin_lock_unfair(lock); > >>>>+ else > >>>>+ queue_spin_lock(lock); > >>>>+} > >>>So I would have expected something like: > >>> > >>> if (static_key_false(&paravirt_spinlock)) { &g...

...ng wrote: > >>On 03/13/2014 11:15 AM, Peter Zijlstra wrote: > >>>On Wed, Mar 12, 2014 at 02:54:52PM -0400, Waiman Long wrote: > >>>>+static inline void arch_spin_lock(struct qspinlock *lock) > >>>>+{ > >>>>+ if (static_key_false(&paravirt_unfairlocks_enabled)) > >>>>+ queue_spin_lock_unfair(lock); > >>>>+ else > >>>>+ queue_spin_lock(lock); > >>>>+} > >>>So I would have expected something like: > >>> > >>> if (static_key_false(&paravirt_spinlock)) { &g...

v4->v5: - Move the optimized 2-task contending code to the generic file to enable more architectures to use it without code duplication. - Address some of the style-related comments by PeterZ. - Allow the use of unfair queue spinlock in a real para-virtualized execution environment. - Add para-virtualization support to the qspinlock code by ensuring that the lock holder and queue