Re: [PATCH 00/29] crypto: talitos - Driver cleanup

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 01/06/2026 à 11:17, Paul Louvel a écrit :
>>> The Freescale Integrated Security Engine (SEC) aka "Talitos" driver
>>> implementation is a monolithic ~3800-line file that mixes SEC1 and SEC2
>>> hardware variants with hash, skcipher, aead and hwrng algorithm.
>>>
>>> This series reorganises the driver to improve readability and
>>> maintainability.
>>
>> Did you analyse the cost of this series ? bloat-o-meter gives the
>> following result, allthough I'm a bit surprised there are only added
>> items, no removed items:
> 
> When you say 'cost', do you mean cost in terms of code size ? performance cost ?
> or both ?
> Regarding code size, I trusted the differences shown in the cover letter by git:
> 
>> 13 files changed, 3810 insertions(+), 3707 deletions(-)

No I mean cost in terms of text size, and cost in terms of execution flow.

> 
> There is 103 insertions more than deletions. This is due to the fact that
> splitting up SEC1/SEC2 code requires additional function and structures.
> I find it acceptable given the readability improvement.
> 
> As for performance, I ran ftrace with the function graph tracer, hashing a 100kb
> file.

The thing is you are doing a test in an ideal world. But the cost can be 
a lot higher on a busy board where you have to consider task switches, 
cache loading, etc ....
The best is to look at the object text. The more flat it is the most 
efficient it is. No branching, no function calls in fast pathes.

When you take a function like this:

00001b14 <to_talitos_ptr>:
     1b14:       2c 06 00 00     cmpwi   r6,0
     1b18:       90 83 00 04     stw     r4,4(r3)
     1b1c:       41 82 00 0c     beq     1b28 <to_talitos_ptr+0x14>
     1b20:       b0 a3 00 02     sth     r5,2(r3)
     1b24:       4e 80 00 20     blr
     1b28:       b0 a3 00 00     sth     r5,0(r3)
     1b2c:       98 c3 00 03     stb     r6,3(r3)
     1b30:       4e 80 00 20     blr

You see that is case is_sec1 is 1 it is just two stores, when is_sec1 is 
0 it is three stores. So inlining those two/three stores at build is 
definitely more efficient than having such a function.

And it is even worth with that one:

00001b84 <to_talitos_ptr_ext_set>:
     1b84:       2c 05 00 00     cmpwi   r5,0
     1b88:       4c 82 00 20     bnelr
     1b8c:       98 83 00 02     stb     r4,2(r3)
     1b90:       4e 80 00 20     blr

If is_sec1 is not 0 the function does nothing, so all the cost of a 
function call to do nothing at the end.


> 
> It looks like there is a slight performance penalty with ahash_finup().
> Otherwise, there is a slight performance improvement for the other measurements.
> I do not know if there is a better way to measure the performance impact of this
> series. If you know, do not hesitate to share it to me.

With experience, the best way to evaluate performance impact is ... look 
at generated object text.

> 
> 
> Best regards,
> Paul.
>