Re: [PATCH v2 08/20] s390/zcrypt: Introduce pre-allocated device status array for cca misc

[Holger Dengler <dengler@linux.ibm.com>]

On 04/03/2025 18:21, Harald Freudenberger wrote:
> Introduce a pre-allocated device status array memory together with
> a mutex controlling the occupation to be used by the two findcard()
> functions. Limit the device status array to max 128 cards and max

As far as I can see, cca_findcard() (and also the static findcard()) are no longer used. If this is the case, I would prefer to remove this unused variant, either in this patch (with a hint in the commit message) or in a separate one (in the order prior to this patch). 

> 128 domains to reduce the size of this pre-allocated memory to 64 KB.

I'm a bit unhappy about introducing a synchronization with this single piece of memory. 

> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> ---
>  drivers/s390/crypto/zcrypt_ccamisc.c | 78 +++++++++++++++++++---------
>  1 file changed, 53 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_ccamisc.c b/drivers/s390/crypto/zcrypt_ccamisc.c
> index 420895df70f0..65b4cdb9b478 100644
> --- a/drivers/s390/crypto/zcrypt_ccamisc.c
> +++ b/drivers/s390/crypto/zcrypt_ccamisc.c
[...]
> @@ -1801,21 +1817,20 @@ static int findcard(u64 mkvp, u16 *pcardnr, u16 *pdomain,

As mentioned above, please check for users of findcard() and cca_findcard(). If there are no users anymore, please remove the obsolete code.

[...]
> @@ -2023,11 +2041,21 @@ int __init zcrypt_ccamisc_init(void)
>  		cprb_mempool = NULL;
>  		return -ENOMEM;
>  	}
> +
> +	/* Pre-allocate one crypto status card struct used in findcard() */
> +	dev_status_mem = kvmalloc(ZCRYPT_DEV_STATUS_EXT_SIZE, GFP_KERNEL);

The alloc of dev_status_mem should be protected by the mutex as well.

> +	if (!dev_status_mem) {
> +		ZCRYPT_DBF_ERR("%s allocation of dev_status_mem failed\n", __func__);
> +		mempool_destroy(cprb_mempool);
> +		return -ENOMEM;
> +	}
> +
>  	return 0;
>  }
>  
>  void zcrypt_ccamisc_exit(void)
>  {
>  	mkvp_cache_free();
> +	kvfree(dev_status_mem);

Same here: please lock the mutex before releasing dev_status_mem.

>  	mempool_destroy(cprb_mempool);
>  }

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com