Received-SPF: pass (google.com: domain of linux-kernel+bounces-97160-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
Date: Fri, 08 Mar 2024 15:19:14 +0100
From: Harald Freudenberger <freude@linux.ibm.com>
To: "Ricardo B. Marliere" <ricardo@marliere.net>,
        Heiko Carstens
 <hca@linux.ibm.com>, Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
        Christian Borntraeger
 <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman
 <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant
Reply-To: freude@linux.ibm.com
Mail-Reply-To: freude@linux.ibm.com
In-Reply-To: <20240305-class_cleanup-s390-v1-1-c4ff1ec49ffd@marliere.net>
References: <20240305-class_cleanup-s390-v1-0-c4ff1ec49ffd@marliere.net>
 <20240305-class_cleanup-s390-v1-1-c4ff1ec49ffd@marliere.net>
Message-ID: <84263def1d38584cd83558a33bb52f22@linux.ibm.com>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit

On 2024-03-05 12:25, Ricardo B. Marliere wrote:
> Since commit 43a7206b0963 ("driver core: class: make class_register() 
> take
> a const *"), the driver core allows for struct class to be in read-only
> memory, so move the zcrypt_class structure to be declared at build time
> placing it into read-only memory, instead of having to be dynamically
> allocated at boot time.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
> ---
>  drivers/s390/crypto/zcrypt_api.c | 33 
> +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_api.c 
> b/drivers/s390/crypto/zcrypt_api.c
> index e8742757085b..d0358bb6ccf2 100644
> --- a/drivers/s390/crypto/zcrypt_api.c
> +++ b/drivers/s390/crypto/zcrypt_api.c
> @@ -116,7 +116,11 @@ EXPORT_SYMBOL(zcrypt_msgtype);
> 
>  struct zcdn_device;
> 
> -static struct class *zcrypt_class;
> +static void zcdn_device_release(struct device *dev);
> +static const struct class zcrypt_class = {
> +	.name = ZCRYPT_NAME,
> +	.dev_release = zcdn_device_release,
> +};
>  static dev_t zcrypt_devt;
>  static struct cdev zcrypt_cdev;
> 
> @@ -139,7 +143,7 @@ static int zcdn_destroy(const char *name);
>   */
>  static inline struct zcdn_device *find_zcdndev_by_name(const char 
> *name)
>  {
> -	struct device *dev = class_find_device_by_name(zcrypt_class, name);
> +	struct device *dev = class_find_device_by_name(&zcrypt_class, name);
> 
>  	return dev ? to_zcdn_dev(dev) : NULL;
>  }
> @@ -151,7 +155,7 @@ static inline struct zcdn_device
> *find_zcdndev_by_name(const char *name)
>   */
>  static inline struct zcdn_device *find_zcdndev_by_devt(dev_t devt)
>  {
> -	struct device *dev = class_find_device_by_devt(zcrypt_class, devt);
> +	struct device *dev = class_find_device_by_devt(&zcrypt_class, devt);
> 
>  	return dev ? to_zcdn_dev(dev) : NULL;
>  }
> @@ -405,7 +409,7 @@ static int zcdn_create(const char *name)
>  		goto unlockout;
>  	}
>  	zcdndev->device.release = zcdn_device_release;
> -	zcdndev->device.class = zcrypt_class;
> +	zcdndev->device.class = &zcrypt_class;
>  	zcdndev->device.devt = devt;
>  	zcdndev->device.groups = zcdn_dev_attr_groups;
>  	if (name[0])
> @@ -2067,12 +2071,9 @@ static int __init zcdn_init(void)
>  	int rc;
> 
>  	/* create a new class 'zcrypt' */
> -	zcrypt_class = class_create(ZCRYPT_NAME);
> -	if (IS_ERR(zcrypt_class)) {
> -		rc = PTR_ERR(zcrypt_class);
> +	rc = class_register(&zcrypt_class);
> +	if (rc)
>  		goto out_class_create_failed;
> -	}
> -	zcrypt_class->dev_release = zcdn_device_release;
> 
>  	/* alloc device minor range */
>  	rc = alloc_chrdev_region(&zcrypt_devt,
> @@ -2088,35 +2089,35 @@ static int __init zcdn_init(void)
>  		goto out_cdev_add_failed;
> 
>  	/* need some class specific sysfs attributes */
> -	rc = class_create_file(zcrypt_class, &class_attr_zcdn_create);
> +	rc = class_create_file(&zcrypt_class, &class_attr_zcdn_create);
>  	if (rc)
>  		goto out_class_create_file_1_failed;
> -	rc = class_create_file(zcrypt_class, &class_attr_zcdn_destroy);
> +	rc = class_create_file(&zcrypt_class, &class_attr_zcdn_destroy);
>  	if (rc)
>  		goto out_class_create_file_2_failed;
> 
>  	return 0;
> 
>  out_class_create_file_2_failed:
> -	class_remove_file(zcrypt_class, &class_attr_zcdn_create);
> +	class_remove_file(&zcrypt_class, &class_attr_zcdn_create);
>  out_class_create_file_1_failed:
>  	cdev_del(&zcrypt_cdev);
>  out_cdev_add_failed:
>  	unregister_chrdev_region(zcrypt_devt, ZCRYPT_MAX_MINOR_NODES);
>  out_alloc_chrdev_failed:
> -	class_destroy(zcrypt_class);
> +	class_unregister(&zcrypt_class);
>  out_class_create_failed:
>  	return rc;
>  }
> 
>  static void zcdn_exit(void)
>  {
> -	class_remove_file(zcrypt_class, &class_attr_zcdn_create);
> -	class_remove_file(zcrypt_class, &class_attr_zcdn_destroy);
> +	class_remove_file(&zcrypt_class, &class_attr_zcdn_create);
> +	class_remove_file(&zcrypt_class, &class_attr_zcdn_destroy);
>  	zcdn_destroy_all();
>  	cdev_del(&zcrypt_cdev);
>  	unregister_chrdev_region(zcrypt_devt, ZCRYPT_MAX_MINOR_NODES);
> -	class_destroy(zcrypt_class);
> +	class_unregister(&zcrypt_class);
>  }
> 
>  /*

Thanks Ricardo, nice work.
The only thing I would do is to rename the label 
"out_class_create_failed"
with "out_class_register_failed".

Who will pick this patch? As this is part of a bundle of fixes, Richardo
do you have a way to push this into the kernel? Otherwise as the 
AP/zcrypt
maintainer I would pick only this patch and forward it to the s390 
subsystem.

Acked-by: Harald Freudenberger <freude@linux.ibm.com>