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

[George Spelvin <lkml@SDF.ORG>]

On Wed, Mar 18, 2020 at 12:40:33PM -0700, Dan Williams wrote:
> Yes, editorializing on unlikely(). Specifically I would normally ask
> for perf numbers to show that the hint is worth it, but I talked
> myself out of asking for that in this case.

Ah, now I understand!  Yes, it's just gilding the lily, but as long as I 
was messing about in the area it seemed worth adding.

You're saying that if the code were a hotter code path, you'd want 
benchmarks, but given its actual infrequent usage, you're satisfied with 
the argument that it's probably right; it's not like we've crippled the 
kernel even if wrong.

> I'm referring to this:
> 
>       if (is_shuffle_order(order))
>                 add_to_free_area_random(page, &zone->free_area[order],
> 
> Where shuffle order is MAX_ORDER-1. I.e. this code is only triggered
> when we might be releasing a 4MB buddy-page.

Ding!  Okay, now it makes sense.  I didn't look that far up the call
stack.  I was auditing each and every call to a get_random function in
the kernel, when I came across this call site, struggled to understand
what it was doing, and rewrote it to be less wrong.

I only vaguely understand where it fits into the larger mm/ system
so I never noticed that condition on its use.

Yes, given that infrequent use, I'm even happier that I focused on code
size rather than performance.

Thank you for taking the time to explain.