Re: [PATCH v2] mm/shuffle.c: Fix races in add_to_free_area_random()

[George Spelvin <lkml@SDF.ORG>]

On Tue, Mar 17, 2020 at 08:53:55PM -0700, Dan Williams wrote:
> On Tue, Mar 17, 2020 at 6:44 PM George Spelvin <lkml@sdf.org> wrote:
>> -       if (rand_bits == 0) {
>> -               rand_bits = 64;
>> -               rand = get_random_u64();
>> +       if (unlikely(rshift == 0)) {
> 
> I had the impression that unless unlikely is "mostly never" then it
> can do more harm than good. Is a branch guaranteed to be taken every
> BITS_PER_LONG'th occurrence really a candidate for unlikely()
> annotation?

I had to look this up.  GCC manual:

For the purposes of branch prediction optimizations, the probability
that a '__builtin_expect' expression is 'true' is controlled by GCC's
'builtin-expect-probability' parameter, which defaults to 90%.  You can
also use '__builtin_expect_with_probability' to explicitly assign a
probability value to individual expressions.

So I think that <= 10% is good enough, which is true in this case.

I was tring to encourage the compiler to:
* Place this code path out of line, and
* Not do the stack manipulations (build a frame, spill registers)
  needed for a non-leaf function if this path isn't taken.