From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv1] arm: reduce power use by contented spin locks with WFE/SEV Date: Fri, 31 Jul 2015 13:43:09 +0100 Message-ID: <55BB6D5D.1010207@citrix.com> References: <1438339545-22400-1-git-send-email-david.vrabel@citrix.com> <55BB537E.9050407@citrix.com> <55BB558D.10808@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZL9ej-000103-Kd for xen-devel@lists.xenproject.org; Fri, 31 Jul 2015 12:43:21 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , Julien Grall Cc: Ian Campbell , Andrew Cooper , Tim Deegan , David Vrabel , Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 31/07/15 12:41, Stefano Stabellini wrote: > On Fri, 31 Jul 2015, Julien Grall wrote: >> Hi David, >> >> On 31/07/15 11:52, David Vrabel wrote: >>> On 31/07/15 11:45, David Vrabel wrote: >>>> Instead of cpu_relax() while spinning and observing the ticket head, >>>> introduce spin_relax() which executes a WFE instruction. After the >>>> ticket head is changed call spin_signal() to execute an SVE >>>> instruction to wake any spinners. >>>> >>>> This should improve power consumption when locks are contented and >>>> spinning. >>>> >>>> Signed-off-by: David Vrabel >>>> --- >>>> I've not tested this but it looks straight-forward... >>>> --- >>>> xen/common/spinlock.c | 5 +++-- >>>> xen/include/asm-arm/spinlock.h | 3 ++- >>>> xen/include/asm-x86/spinlock.h | 3 +++ >>>> 3 files changed, 8 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c >>>> index 29149d1..fc3e8e7 100644 >>>> --- a/xen/common/spinlock.c >>>> +++ b/xen/common/spinlock.c >>>> @@ -141,7 +141,7 @@ void _spin_lock(spinlock_t *lock) >>>> while ( tickets.tail != observe_head(&lock->tickets) ) >>>> { >>>> LOCK_PROFILE_BLOCK; >>>> - cpu_relax(); >>>> + spin_relax(); >>>> } >>>> LOCK_PROFILE_GOT; >>>> preempt_disable(); >>>> @@ -170,6 +170,7 @@ void _spin_unlock(spinlock_t *lock) >>>> preempt_enable(); >>>> LOCK_PROFILE_REL; >>>> add_sized(&lock->tickets.head, 1); >>>> + spin_signal(); >>> >>> It occurs to me that perhaps there should be a barrier between the >>> add_sized() and the spin_signal() so the update value is visible before >>> we signal (otherwise the spinner may be woken and observe the old value >>> and WFE again). >> >> sev is usually precede by dsb to ensure that all the instructions before >> as completed before executing the sev. > > Yes, a dsb() is required. This being common code, we could use wmb(). You should put the barrier required for the SEV in spin_signal() so an additional barrier is not required on other archs. David