Re: [PATCH v2 05/20] s390/zcrypt: Introduce cprb mempool for cca misc functions

Harald Freudenberger
2025-03-25T08:27:00Z

On 2025-03-18 15:16, Holger Dengler wrote:
> On 04/03/2025 18:21, Harald Freudenberger wrote:
>> Introduce a cprb mempool for the zcrypt cca misc functions
>> (zcrypt_ccamisc.*) do some preparation rework to support
>> a do-not-allocate path through some zcrypt cca misc functions.
>>
>> The mempool is by default space for 4 cprbs with each 16KB.
>> For CCA a CPRB needs to hold the request and the reply, so
>> the pool items only support requests/replies with a limit of
>> about 8KB. The minimal number of items in the mempool can get
>> adjusted via module parameter zcrypt.cca_cprbpool_min_items
>> on module load.
>>
>> This is only part of an rework to support a new xflag
>> ZCRYPT_XFLAG_NOMEMALLOC but not yet complete.
>>
>> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
>
> One minor comment below. Beside that
> Reviewed-by: Holger Dengler <dengler@linux.ibm.com>
>
>> ---
>> drivers/s390/crypto/zcrypt_api.c | 13 ++-
>> drivers/s390/crypto/zcrypt_api.h | 2 +
>> drivers/s390/crypto/zcrypt_ccamisc.c | 116
>> ++++++++++++++++++++-------
>> drivers/s390/crypto/zcrypt_ccamisc.h | 1 +
>> 4 files changed, 101 insertions(+), 31 deletions(-)
>>
> [...]
>> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c
>> b/drivers/s390/crypto/zcrypt_ccamisc.c
>> index 521baaea06ff..2c2ca2d67e2d 100644
>> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
>> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
> [...]
>> @@ -229,7 +241,15 @@ static int alloc_and_prep_cprbmem(size_t
>> paramblen,
>> * allocate consecutive memory for request CPRB, request param
>> * block, reply CPRB and reply param block
>> */
>> - cprbmem = kcalloc(2, cprbplusparamblen, GFP_KERNEL);
>> + if (xflags & ZCRYPT_XFLAG_NOMEMALLOC) {
>> + if (cprb_mempool &&
>> + 2 * cprbplusparamblen <= CPRB_MEMPOOL_ITEM_SIZE) {
>> + cprbmem = mempool_alloc_preallocated(cprb_mempool);
>> + memset(cprbmem, 0, 2 * cprbplusparamblen);
>> + }
>> + } else {
>
> Just a nit: let the compiler decide, if code should be duplicated or
> not.
>
> if (xflags & ZCRYPT_XFLAG_NOMEMALLOC) {
> size_t len = 2 * cprbplusparamblen;
>
> if (cprb_mempool && len <= CPRB_MEMPOOL_ITEM_SIZE) {
> cprbmem = mempool_alloc_preallocated(cprb_mempool);
> memset(cprbmem, 0, len);
> }
> } else {
>
>> + cprbmem = kcalloc(2, cprbplusparamblen, GFP_KERNEL);
>> + }
>> if (!cprbmem)
>> return -ENOMEM;
>>
> [...]

It was not my intention to outsmart the compier when writing this code.
However, changed to your suggestion -> v3