Re: [PATCH v3 2/3] prandom: Convert prandom_u32_state() to __always_inline

Ryan Roberts
2026-01-05T10:34:46Z

On 03/01/2026 10:46, David Laight wrote:
> On Fri, 2 Jan 2026 14:09:26 +0000
> Ryan Roberts <ryan.roberts@arm.com> wrote:
>
>> On 02/01/2026 13:39, Jason A. Donenfeld wrote:
>>> Hi Ryan,
>>>
>>> On Fri, Jan 2, 2026 at 2:12 PM Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>> context. Given the function is just a handful of operations and doesn't
>>>
>>> How many? What's this looking like in terms of assembly?
>>
>> 25 instructions on arm64:
>>
>> 0000000000000000 <prandom_u32_state>:
>> 0: 29401403 ldp w3, w5, [x0]
>> 4: aa0003e1 mov x1, x0
>> 8: 29410002 ldp w2, w0, [x0, #8]
>> c: 531e74a4 lsl w4, w5, #2
>> 10: 530e3468 lsl w8, w3, #18
>> 14: 4a0400a5 eor w5, w5, w4
>> 18: 4a031863 eor w3, w3, w3, lsl #6
>> 1c: 53196047 lsl w7, w2, #7
>> 20: 53134806 lsl w6, w0, #13
>> 24: 4a023442 eor w2, w2, w2, lsl #13
>> 28: 4a000c00 eor w0, w0, w0, lsl #3
>> 2c: 121b6884 and w4, w4, #0xffffffe0
>> 30: 120d3108 and w8, w8, #0xfff80000
>> 34: 121550e7 and w7, w7, #0xfffff800
>> 38: 120c2cc6 and w6, w6, #0xfff00000
>> 3c: 2a456c85 orr w5, w4, w5, lsr #27
>> 40: 2a433504 orr w4, w8, w3, lsr #13
>> 44: 2a4254e3 orr w3, w7, w2, lsr #21
>> 48: 2a4030c2 orr w2, w6, w0, lsr #12
>> 4c: 4a020066 eor w6, w3, w2
>> 50: 4a050080 eor w0, w4, w5
>> 54: 4a0000c0 eor w0, w6, w0
>> 58: 29001424 stp w4, w5, [x1]
>> 5c: 29010823 stp w3, w2, [x1, #8]
>> 60: d65f03c0 ret
>
> That is gcc, clang seems to generate something horrid (from godbolt).
> I'm not sure what it has tried to do (and maybe it can't in kernel)
> but it clearly doesn't help!
> .LCPI0_0:
> .word 18
> .word 2
> .word 7
> .word 13
> .LCPI0_1:
> .word 6
> .word 2
> .word 13
> .word 3
> .LCPI0_2:
> .word 4294443008
> .word 4294967264
> .word 4294965248
> .word 4293918720
> .LCPI0_3:
> .word 4294967283
> .word 4294967269
> .word 4294967275
> .word 4294967284
> prandom_u32_state:
> adrp x9, .LCPI0_1
> ldr q0, [x0]
> adrp x10, .LCPI0_3
> ldr q1, [x9, :lo12:.LCPI0_1]
> adrp x9, .LCPI0_0
> ldr q3, [x10, :lo12:.LCPI0_3]
> ldr q2, [x9, :lo12:.LCPI0_0]
> adrp x9, .LCPI0_2
> mov x8, x0
> ushl v1.4s, v0.4s, v1.4s
> ushl v2.4s, v0.4s, v2.4s
> eor v0.16b, v1.16b, v0.16b
> ldr q1, [x9, :lo12:.LCPI0_2]
> and v1.16b, v2.16b, v1.16b
> ushl v0.4s, v0.4s, v3.4s
> orr v0.16b, v0.16b, v1.16b
> ext v1.16b, v0.16b, v0.16b, #8
> str q0, [x8]
> eor v1.8b, v0.8b, v1.8b
> fmov x9, d1
> lsr x10, x9, #32
> eor w0, w9, w10
> ret
>
> The x86 versions are a little longer (arm's barrel shifter helps a lot).
>
>>
>>> It'd also be
>>> nice to have some brief analysis of other call sites to have
>>> confirmation this isn't blowing up other users.
>>
>> I compiled defconfig before and after this patch on arm64 and compared the text
>> sizes:
>>
>> $ ./scripts/bloat-o-meter -t vmlinux.before vmlinux.after
>> add/remove: 3/4 grow/shrink: 4/1 up/down: 836/-128 (708)
>> Function old new delta
>> prandom_seed_full_state 364 932 +568
>> pick_next_task_fair 1940 2036 +96
>> bpf_user_rnd_u32 104 196 +92
>> prandom_bytes_state 204 260 +56
>> e843419@0f2b_00012d69_e34 - 8 +8
>> e843419@0db7_00010ec3_23ec - 8 +8
>> e843419@02cb_00003767_25c - 8 +8
>> bpf_prog_select_runtime 448 444 -4
>> e843419@0aa3_0000cfd1_1580 8 - -8
>> e843419@0aa2_0000cfba_147c 8 - -8
>> e843419@075f_00008d8c_184 8 - -8
>> prandom_u32_state 100 - -100
>> Total: Before=19078072, After=19078780, chg +0.00%
>>
>> So 708 bytes more after inlining.
>
> Doesn't look like there are many calls.
>
>> The main cost is prandom_seed_full_state(),
>> which calls prandom_u32_state() 10 times (via prandom_warmup()). I expect we
>> could turn that into a loop to reduce ~450 bytes overall.
>
> That would always have helped the code size.
> And I suspect the other costs of that code make unrolling the loop pointless.
>
>>
>> I'm not really sure if 708 is good or bad...
>>
>>>
>>>> +static __always_inline u32 prandom_u32_state(struct rnd_state *state)
>>>
>>> Why not just normal `inline`? Is gcc disagreeing with the inlinability
>>> of this function?
>>
>> Given this needs to be called from a noinstr function, I didn't want to give the
>> compiler the opportunity to decide not to inline it, since in that case, some
>> instrumentation might end up being applied to the function body which would blow
>> up when called in the noinstr context.
>>
>> I think the other 2 options are to keep prandom_u32_state() in the c file but
>> mark it noinstr or rearrange all the users so that thay don't call it until
>> instrumentation is allowable. The latter is something I was trying to avoid.
>>
>> There is some previous discussion of this at [1].
>>
>> [1] https://lore.kernel.org/all/aS65LFUfdgRPKv1l@J2N7QTR9R3/
>>
>> Perhaps keeping prandom_u32_state() in the c file and making it noinstr is the
>> best compromise?
>
> Or define prandom_u32_state_inline() as always_inline and have the
> real function:
> u32 prandom_u32_state(struct rnd_state *state)
> {
> return prandom_u32_state_inline(state);
> }
>
> So that the callers can pick the inline version if it really matters.

Ahh yes, that sounds like the simplest/best idea to me. I'll take this approach
for the next version assuming Jason is ok with it?

Thanks,
Ryan

>
> David
>
>>
>> Thanks,
>> Ryan
>>
>>>
>>> Jason
>>
>>
>