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 14:42, <Ian.Campbell@xxxxxxxxxx> wrote:
> On Mon, 2014-04-07 at 13:36 +0100, Jan Beulich wrote:
>> >>> 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,
> 
> 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... <etc as before>
> ?

Yes, that reads fine for both -unstable and the backport.

Jan


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