Re: [PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
On 08/08/2015 02:02 AM, Peter Zijlstra wrote: On Fri, Aug 07, 2015 at 11:17:56PM -0400, Waiman Long wrote: If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. The extra check at the end of __pv_queued_spin_unlock() is unnecessary and so is removed. This is half the patch it should be. Because if the load is not needed, then the store is not either, and then there's the comments to update. Sorry for the late reply. That is true. Setting state to vcpu_hashed in pv_wait_head() isn't really necessary. I kept it there for consistency sake as the state may be set to vcpu_hashed in pv_kick_node(). We can certainly take that out. BTW, could you also review the other patches when you have time? I am coming to the LinuxCon/Plumbers next week. Hopefully, I can chat with you again. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
On 08/08/2015 02:02 AM, Peter Zijlstra wrote: On Fri, Aug 07, 2015 at 11:17:56PM -0400, Waiman Long wrote: If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. The extra check at the end of __pv_queued_spin_unlock() is unnecessary and so is removed. This is half the patch it should be. Because if the load is not needed, then the store is not either, and then there's the comments to update. Sorry for the late reply. That is true. Setting state to vcpu_hashed in pv_wait_head() isn't really necessary. I kept it there for consistency sake as the state may be set to vcpu_hashed in pv_kick_node(). We can certainly take that out. BTW, could you also review the other patches when you have time? I am coming to the LinuxCon/Plumbers next week. Hopefully, I can chat with you again. Cheers, Longman -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
On Fri, Aug 07, 2015 at 11:17:56PM -0400, Waiman Long wrote: > If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. > The extra check at the end of __pv_queued_spin_unlock() is unnecessary > and so is removed. This is half the patch it should be. Because if the load is not needed, then the store is not either, and then there's the comments to update. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
On Fri, Aug 07, 2015 at 11:17:56PM -0400, Waiman Long wrote: If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. The extra check at the end of __pv_queued_spin_unlock() is unnecessary and so is removed. This is half the patch it should be. Because if the load is not needed, then the store is not either, and then there's the comments to update. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. The extra check at the end of __pv_queued_spin_unlock() is unnecessary and so is removed. Signed-off-by: Waiman Long Reviewed-by: Davidlohr Bueso --- kernel/locking/qspinlock_paravirt.h |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h index c8e6e9a..6eafb9e 100644 --- a/kernel/locking/qspinlock_paravirt.h +++ b/kernel/locking/qspinlock_paravirt.h @@ -364,8 +364,7 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock) * vCPU is harmless other than the additional latency in completing * the unlock. */ - if (READ_ONCE(node->state) == vcpu_hashed) - pv_kick(node->cpu); + pv_kick(node->cpu); } /* * Include the architecture specific callee-save thunk of the -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 1/6] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL
If _Q_SLOW_VAL has been set, the vCPU state must have been vcpu_hashed. The extra check at the end of __pv_queued_spin_unlock() is unnecessary and so is removed. Signed-off-by: Waiman Long waiman.l...@hp.com Reviewed-by: Davidlohr Bueso d...@stgolabs.net --- kernel/locking/qspinlock_paravirt.h |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h index c8e6e9a..6eafb9e 100644 --- a/kernel/locking/qspinlock_paravirt.h +++ b/kernel/locking/qspinlock_paravirt.h @@ -364,8 +364,7 @@ __visible void __pv_queued_spin_unlock(struct qspinlock *lock) * vCPU is harmless other than the additional latency in completing * the unlock. */ - if (READ_ONCE(node-state) == vcpu_hashed) - pv_kick(node-cpu); + pv_kick(node-cpu); } /* * Include the architecture specific callee-save thunk of the -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
