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

Holger Dengler
2025-03-19T14:32:00Z

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