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

["Paul Louvel" <paul.louvel@bootlin.com>]

>> 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(-)

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.

Without the series applied:

be935f36ae14489758e28a83cfec418d3ad600b64628166f275c7ae6aac7b9af  ./test_100k.bin
# tracer: function_graph
#
# CPU  DURATION                  FUNCTION CALLS
# |     |   |                     |   |   |   |
 0) + 20.256 us   |  ahash_init();
 0)               |  ahash_do_req_chain() {
 0)               |    ahash_update() {
 0) + 41.088 us   |      ahash_process_req();
 0) + 54.272 us   |    }
 0) + 61.536 us   |  }
 ------------------------------------------
 0)  sha256s-196   =>    <idle>-0   
 ------------------------------------------

 0) + 45.248 us   |  ahash_done();
 ------------------------------------------
 0)    <idle>-0    =>  sha256s-196  
 ------------------------------------------

 0)               |  ahash_do_req_chain() {
 0)               |    ahash_update() {
 0) + 39.552 us   |      ahash_process_req();
 0) + 53.472 us   |    }
 0) + 68.576 us   |  }
 ------------------------------------------
 0)  sha256s-196   =>    <idle>-0   
 ------------------------------------------

 0) + 39.680 us   |  ahash_done();
 ------------------------------------------
 0)    <idle>-0    =>  sha256s-196  
 ------------------------------------------

 0)               |  ahash_do_req_chain() {
 0)               |    ahash_finup() {
 0)               |      ahash_process_req() {
 0) + 16.800 us   |        ahash_done();
 0) + 96.192 us   |      }
 0) ! 103.616 us  |    }
 0) ! 121.344 us  |  }

With the series applied:

be935f36ae14489758e28a83cfec418d3ad600b64628166f275c7ae6aac7b9af  ./test_100k.bin
# tracer: function_graph
#
# CPU  DURATION                  FUNCTION CALLS
# |     |   |                     |   |   |   |
 0) + 20.576 us   |  ahash_init();
 0)               |  ahash_do_req_chain() {
 0)               |    ahash_update() {
 0) + 32.896 us   |      ahash_process_req();
 0) + 46.688 us   |    }
 0) + 54.016 us   |  }
 ------------------------------------------
 0)  sha256s-196   =>    <idle>-0   
 ------------------------------------------

 0)               |  ahash_done() {
 0)               |    ahash_update_done() {
 0)   9.312 us    |      ahash_update_finish();
 0) + 44.416 us   |    }
 0) + 73.216 us   |  }
 ------------------------------------------
 0)    <idle>-0    =>  sha256s-196  
 ------------------------------------------

 0)               |  ahash_do_req_chain() {
 0)               |    ahash_update() {
 0) + 33.120 us   |      ahash_process_req();
 0) + 46.912 us   |    }
 0) + 61.664 us   |  }
 ------------------------------------------
 0)  sha256s-196   =>    <idle>-0   
 ------------------------------------------

 0)               |  ahash_done() {
 0)               |    ahash_update_done() {
 0)   8.928 us    |      ahash_update_finish();
 0) + 42.720 us   |    }
 0) + 69.440 us   |  }
 ------------------------------------------
 0)    <idle>-0    =>  sha256s-196  
 ------------------------------------------

 0)               |  ahash_do_req_chain() {
 0)               |    ahash_finup() {
 0)               |      ahash_process_req() {
 0)               |        ahash_done() {
 0)   5.696 us    |          ahash_finup_done();
 0) + 29.760 us   |        }
 0) ! 107.168 us  |      }
 0) ! 113.696 us  |    }
 0) ! 131.840 us  |  }

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.


Best regards,
Paul.

-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com