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 11:52:46 +0100 Message-ID: <55BB537E.9050407@citrix.com> References: <1438339545-22400-1-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZL7vw-0001LL-IC for xen-devel@lists.xenproject.org; Fri, 31 Jul 2015 10:53:00 +0000 In-Reply-To: <1438339545-22400-1-git-send-email-david.vrabel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, Andrew Cooper , Tim Deegan , Jan Beulich , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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). spin_relax() and spin_signal() might be better named arch_lock_relax() and arch_lock_signal() to match the naming of existing arch_lock_*() hooks. I think someone with more arm experience (and the means to test) should take this patch on. David