Re: [Xen-devel] [PATCH] xen: make sure that likely and unlikely convert the expression to a boolean

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.04.14 at 13:07, <ian.campbell@xxxxxxxxxx> 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, 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.

I wonder how we managed to miss the similar change in Linux - even
2.6.5 already has it that way.

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- 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))



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel