From patchwork Thu Apr 10 11:37:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Campbell X-Patchwork-Id: 28155 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ig0-f197.google.com (mail-ig0-f197.google.com [209.85.213.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A7E12212DC for ; Thu, 10 Apr 2014 11:38:59 +0000 (UTC) Received: by mail-ig0-f197.google.com with SMTP id hn18sf10204487igb.4 for ; Thu, 10 Apr 2014 04:38:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:from:to:date:in-reply-to :references:organization:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-post:list-help:list-subscribe:sender :errors-to:x-original-sender:x-original-authentication-results :mailing-list:list-archive:content-type:content-transfer-encoding; bh=UX+EHR1DVGIN8UmYXsW+MG607KGhOxPgdI5nelTlUv0=; b=UDyJn4qGMczNtOy5UZ1yW9KEPRCZI/CseGHh3H/EpRpQRYdwYi0jw5t83Au+WwGMUv VrwmmX7sfnGws7YbRI6Yx5MtNjd3j0o/UJLndJ2SI/dn8kA5seZSw7YFGXgBhUzI0KyW EtqVsiE411tQH3goQad012h1z/LfWLjJZUzCocw8Jl1qMWoTo69h4siqf+l2yI2tHbUv s5lhhfHvDdTf+8DYFkZ851PMsXfRZLpzxpTuyNetVUFOm7untjbRN/E4SZDT5TpsqqIJ gUq4e17gfJ31vGK1jstuGcfkgpuB6NI4xmbS5E6LdQgGHGMkDrle2JKH+MigapmSNINF /9Ng== X-Gm-Message-State: ALoCoQkA49+04f1ViH5n+NCeORbOLmKTdfInQHOH1imsa3iCmRXNz1wiWwNePj3d/U+33zSzh5/r X-Received: by 10.182.200.162 with SMTP id jt2mr8053560obc.34.1397129939041; Thu, 10 Apr 2014 04:38:59 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.40.115 with SMTP id w106ls1103528qgw.53.gmail; Thu, 10 Apr 2014 04:38:58 -0700 (PDT) X-Received: by 10.52.95.135 with SMTP id dk7mr283303vdb.32.1397129938832; Thu, 10 Apr 2014 04:38:58 -0700 (PDT) Received: from mail-vc0-f169.google.com (mail-vc0-f169.google.com [209.85.220.169]) by mx.google.com with ESMTPS id kj3si42889vdb.24.2014.04.10.04.38.58 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 10 Apr 2014 04:38:58 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.169 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.169; Received: by mail-vc0-f169.google.com with SMTP id ik5so3302827vcb.14 for ; Thu, 10 Apr 2014 04:38:58 -0700 (PDT) X-Received: by 10.221.26.10 with SMTP id rk10mr14195434vcb.0.1397129938742; Thu, 10 Apr 2014 04:38:58 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.221.72 with SMTP id ib8csp51981vcb; Thu, 10 Apr 2014 04:38:58 -0700 (PDT) X-Received: by 10.140.31.137 with SMTP id f9mr18415591qgf.59.1397129937987; Thu, 10 Apr 2014 04:38:57 -0700 (PDT) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id g2si1742867qab.15.2014.04.10.04.38.57 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Thu, 10 Apr 2014 04:38:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xen.org designates 50.57.142.19 as permitted sender) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WYDIq-0000AF-NH; Thu, 10 Apr 2014 11:37:56 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WYDIp-00009i-4p for xen-devel@lists.xen.org; Thu, 10 Apr 2014 11:37:55 +0000 Received: from [193.109.254.147:45380] by server-10.bemta-14.messagelabs.com id B9/0A-04546-29286435; Thu, 10 Apr 2014 11:37:54 +0000 X-Env-Sender: Ian.Campbell@citrix.com X-Msg-Ref: server-14.tower-27.messagelabs.com!1397129872!7470678!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n X-StarScan-Received: X-StarScan-Version: 6.11.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 15611 invoked from network); 10 Apr 2014 11:37:53 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-14.tower-27.messagelabs.com with RC4-SHA encrypted SMTP; 10 Apr 2014 11:37:53 -0000 X-IronPort-AV: E=Sophos;i="4.97,833,1389744000"; d="scan'208";a="119842090" Received: from accessns.citrite.net (HELO FTLPEX01CL03.citrite.net) ([10.9.154.239]) by FTLPIPO01.CITRIX.COM with ESMTP; 10 Apr 2014 11:37:52 +0000 Received: from [10.80.2.80] (10.80.2.80) by FTLPEX01CL03.citrite.net (10.13.107.80) with Microsoft SMTP Server id 14.2.342.4; Thu, 10 Apr 2014 07:37:51 -0400 Message-ID: <1397129870.9862.140.camel@kazak.uk.xensource.com> From: Ian Campbell To: Jan Beulich Date: Thu, 10 Apr 2014 12:37:50 +0100 In-Reply-To: <5342BC0902000078000062C5@nat28.tlf.novell.com> References: <1396868824-16769-1-git-send-email-ian.campbell@citrix.com> <5342B7DC0200007800006298@nat28.tlf.novell.com> <1396874554.22845.65.camel@kazak.uk.xensource.com> <5342BC0902000078000062C5@nat28.tlf.novell.com> Organization: Citrix Systems, Inc. X-Mailer: Evolution 3.8.5-2+b3 MIME-Version: 1.0 X-Originating-IP: [10.80.2.80] X-DLP: MIA1 Cc: Tim Deegan , keir@xen.org, xen-devel@lists.xen.org Subject: Re: [Xen-devel] [PATCH] xen: make sure that likely and unlikely convert the expression to a boolean X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: ian.campbell@citrix.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.169 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: On Mon, 2014-04-07 at 13:54 +0100, Jan Beulich wrote: > >>> On 07.04.14 at 14:42, wrote: > > On Mon, 2014-04-07 at 13:36 +0100, Jan Beulich wrote: > >> >>> On 07.04.14 at 13:07, wrote: > >> > According to http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html > >> > __builtin_expect has the prototype: > >> > long __builtin_expect (long exp, long c) > >> > > >> > If sizeof(exp) > sizeof(long) then this will effectively mask off the top > > bits > >> > of exp, meaning that the if in "if (unlikey(x))" will see the masked > > version, > >> > which might be false when true was expected, likely has the same issue. > >> > > >> > With the x86_32 hypervisor no longer existing this is mostly likely to > > affect > >> > arm32 builds. A quick grep however shows that all the existing arm32 uses > > of > >> > both likely and unlikely already pass a boolean. I noticed this with an as > > yet > >> > unposted patch which did not have this property. > >> > >> Good catch, > > > > I tore out some hair before I got there of course ;-) > > > >> except that at least in the 4.2 tree we still care for the > >> x86_32 hypervisor, and I already spotted a case having the same > >> issue (in mtrr_var_range_msr_set()). I.e. for the purposes of > >> backporting it might be better to make the statement above a little > >> less tailored to arm32. > > > > That's fine by me. Do you mean for me to extend it now or did you want > > to change it as part of the backport process? Either is fine by me. > > > > How about: > > This is mostly likely to affect x86_32 and arm32 builds. x86_32 > > is not present on 4.3 onwards and a quick grep of current > > staging shows that all the existing arm32... > > ? > > Yes, that reads fine for both -unstable and the backport. Thanks. Here's what I ended up applying: >From e5545fb6d0dc5e2c48b2e450d18246d9bc1ae35b Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Mon, 7 Apr 2014 12:07:04 +0100 Subject: [PATCH] xen: make sure that likely and unlikely convert the expression to a boolean According to http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html __builtin_expect has the prototype: long __builtin_expect (long exp, long c) If sizeof(exp) > sizeof(long) then this will effectively mask off the top bits of exp, meaning that the if in "if (unlikey(x))" will see the masked version, which might be false when true was expected, likely has the same issue. This is mostly likely to affect x86_32 and arm32 builds. x86_32 is not present on 4.3 onwards and a quick grep of current staging shows that all the existing arm32 uses of both likely and unlikely already pass a boolean. I noticed this with an as yet unposted patch which did not have this property. Also the defintion of likely might not have had the expected affect for cases where a true value > 1 might be passed. Signed-off-by: Ian Campbell Reviewed-by: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan --- xen/include/xen/compiler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h index 6e07990..4b3472d 100644 --- a/xen/include/xen/compiler.h +++ b/xen/include/xen/compiler.h @@ -7,8 +7,8 @@ #define barrier() __asm__ __volatile__("": : :"memory") -#define likely(x) __builtin_expect((x),1) -#define unlikely(x) __builtin_expect((x),0) +#define likely(x) __builtin_expect(!!(x),1) +#define unlikely(x) __builtin_expect(!!(x),0) #define inline __inline__ #define always_inline __inline__ __attribute__ ((always_inline))