From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv3 1/4] x86: provide xadd() Date: Tue, 21 Apr 2015 14:15:57 +0100 Message-ID: <55364D8D.4080908@citrix.com> References: <1429611088-23950-1-git-send-email-david.vrabel@citrix.com> <1429611088-23950-2-git-send-email-david.vrabel@citrix.com> <553644530200007800074374@mail.emea.novell.com> <5536444E.2080807@citrix.com> <553668C70200007800074480@mail.emea.novell.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 1YkY2B-0005ms-CZ for xen-devel@lists.xenproject.org; Tue, 21 Apr 2015 13:16:15 +0000 In-Reply-To: <553668C70200007800074480@mail.emea.novell.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: Jan Beulich Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , Andrew Cooper , Jennifer Herbert , Tim Deegan , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 21/04/15 14:12, Jan Beulich wrote: >>>> On 21.04.15 at 14:36, wrote: >> On 21/04/15 11:36, Jan Beulich wrote: >>>>>> On 21.04.15 at 12:11, wrote: >>>> +static always_inline unsigned long __xadd( >>>> + volatile void *ptr, unsigned long v, int size) >>>> +{ >>>> + switch ( size ) >>>> + { >>>> + case 1: >>>> + asm volatile ( "lock; xaddb %b0,%1" >>>> + : "+r" (v), "+m" (*__xg((volatile void *)ptr)) >>>> + :: "memory"); >>>> + return v; >>> >>> This doesn't seem to guarantee to return the old value: When the >>> passed in v has more than 8 significant bits (which will get ignored >>> as input), nothing will zap those bits from the register. Same for >>> the 16-bit case obviously. >>> >>>> +#define xadd(ptr, v) ({ \ >>>> + __xadd((ptr), (unsigned long)(v), sizeof(*(ptr))); \ >>>> + }) >>> >>> Assuming only xadd() is supposed to be used directly, perhaps >>> the easiest would be to cast v to typeof(*(ptr)) (instead of >>> unsigned long) here? >> >> I don't see how this helps. Did you perhaps mean cast the result? >> >> #define xadd(ptr, v) ({ \ >> (typeof *(ptr))__xadd(ptr, (unsigned long)(v), \ >> sizeof(*(ptr))); \ >> }) > > Casting the result would work too; casting the input would have > the same effect because (as said) the actual xadd doesn't alter > bits 8...63 (or 16...63 in the 16-bit case), i.e. whether zero > extension happens before or after doing the xadd doesn't matter. Oh yes, of course. Any preference to which method? David