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

[Harald Freudenberger <freude@linux.ibm.com>]

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