2024-03-05 11:37:11

by Ricardo B. Marliere

[permalink] [raw]
Subject: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant

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 <[email protected]>
Suggested-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Ricardo B. Marliere <[email protected]>
---
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);
}

/*

--
2.43.0



2024-03-08 14:31:44

by Harald Freudenberger

[permalink] [raw]
Subject: Re: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant

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 <[email protected]>
> Suggested-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Ricardo B. Marliere <[email protected]>
> ---
> 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 <[email protected]>


2024-03-08 14:38:31

by Ricardo B. Marliere

[permalink] [raw]
Subject: Re: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant

On 8 Mar 15:19, Harald Freudenberger wrote:
> 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 <[email protected]>
> > Suggested-by: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Ricardo B. Marliere <[email protected]>
> > ---
> > 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".

Ah, indeed. Thanks for catching that. I will wait for more feedback on
the other patches and send a v2 if required.

>
> 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.

I have no ways of pushing this, sorry. The series is based on
s390/linux.git/for-next, so perhaps the s390 maintainers can pick this
one along with the others with your Acked-by: provided? :)

Thank you,
- Ricardo.


>
> Acked-by: Harald Freudenberger <[email protected]>
>

2024-03-08 14:45:04

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant

On Fri, Mar 08, 2024 at 11:38:14AM -0300, Ricardo B. Marliere wrote:
> > Thanks Ricardo, nice work.
> > The only thing I would do is to rename the label "out_class_create_failed"
> > with "out_class_register_failed".
>
> Ah, indeed. Thanks for catching that. I will wait for more feedback on
> the other patches and send a v2 if required.
>
> >
> > 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.
>
> I have no ways of pushing this, sorry. The series is based on
> s390/linux.git/for-next, so perhaps the s390 maintainers can pick this
> one along with the others with your Acked-by: provided? :)

I will pick up the whole series, but need some more time.

There is no need to send a v2 for this patch - I'll change the label as
requested by Harald.

2024-03-08 16:49:20

by Ricardo B. Marliere

[permalink] [raw]
Subject: Re: [PATCH 1/6] s390: zcrypt: make zcrypt_class constant

Hi Heiko!

On 8 Mar 15:44, Heiko Carstens wrote:
> On Fri, Mar 08, 2024 at 11:38:14AM -0300, Ricardo B. Marliere wrote:
> > > Thanks Ricardo, nice work.
> > > The only thing I would do is to rename the label "out_class_create_failed"
> > > with "out_class_register_failed".
> >
> > Ah, indeed. Thanks for catching that. I will wait for more feedback on
> > the other patches and send a v2 if required.
> >
> > >
> > > 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.
> >
> > I have no ways of pushing this, sorry. The series is based on
> > s390/linux.git/for-next, so perhaps the s390 maintainers can pick this
> > one along with the others with your Acked-by: provided? :)
>
> I will pick up the whole series, but need some more time.
>
> There is no need to send a v2 for this patch - I'll change the label as
> requested by Harald.

Thank you for this.
- Ricardo