drm/amdkfd: make kfd_class constant

[PATCH] drm/amdkfd: make kfd_class constant

Posted by Ricardo B. Marliere 1 year, 9 months ago

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 kfd_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/gpu/drm/amd/amdkfd/kfd_chardev.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f030cafc5a0a..dfa8c69532d4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -63,8 +63,10 @@ static const struct file_operations kfd_fops = {
 };
 
 static int kfd_char_dev_major = -1;
-static struct class *kfd_class;
 struct device *kfd_device;
+static const struct class kfd_class = {
+	.name = kfd_dev_name,
+};
 
 static inline struct kfd_process_device *kfd_lock_pdd_by_id(struct kfd_process *p, __u32 gpu_id)
 {
@@ -94,14 +96,13 @@ int kfd_chardev_init(void)
 	if (err < 0)
 		goto err_register_chrdev;
 
-	kfd_class = class_create(kfd_dev_name);
-	err = PTR_ERR(kfd_class);
-	if (IS_ERR(kfd_class))
+	err = class_register(&kfd_class);
+	if (err)
 		goto err_class_create;
 
-	kfd_device = device_create(kfd_class, NULL,
-					MKDEV(kfd_char_dev_major, 0),
-					NULL, kfd_dev_name);
+	kfd_device = device_create(&kfd_class, NULL,
+				   MKDEV(kfd_char_dev_major, 0),
+				   NULL, kfd_dev_name);
 	err = PTR_ERR(kfd_device);
 	if (IS_ERR(kfd_device))
 		goto err_device_create;
@@ -109,7 +110,7 @@ int kfd_chardev_init(void)
 	return 0;
 
 err_device_create:
-	class_destroy(kfd_class);
+	class_unregister(&kfd_class);
 err_class_create:
 	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
 err_register_chrdev:
@@ -118,8 +119,8 @@ int kfd_chardev_init(void)
 
 void kfd_chardev_exit(void)
 {
-	device_destroy(kfd_class, MKDEV(kfd_char_dev_major, 0));
-	class_destroy(kfd_class);
+	device_destroy(&kfd_class, MKDEV(kfd_char_dev_major, 0));
+	class_unregister(&kfd_class);
 	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
 	kfd_device = NULL;
 }

---
base-commit: 8bc75586ea01f1c645063d3472c115ecab03e76c
change-id: 20240305-class_cleanup-drm-amd-bdc7255b7540

Best regards,
-- 
Ricardo B. Marliere <ricardo@marliere.net>

Re: [PATCH] drm/amdkfd: make kfd_class constant

Posted by Felix Kuehling 1 year, 9 months ago

On 2024-03-05 7:15, 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 kfd_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>

The patch looks good to me. Do you want me to apply this to Alex's 
amd-staging-drm-next?

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index f030cafc5a0a..dfa8c69532d4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -63,8 +63,10 @@ static const struct file_operations kfd_fops = {
>   };
>   
>   static int kfd_char_dev_major = -1;
> -static struct class *kfd_class;
>   struct device *kfd_device;
> +static const struct class kfd_class = {
> +	.name = kfd_dev_name,
> +};
>   
>   static inline struct kfd_process_device *kfd_lock_pdd_by_id(struct kfd_process *p, __u32 gpu_id)
>   {
> @@ -94,14 +96,13 @@ int kfd_chardev_init(void)
>   	if (err < 0)
>   		goto err_register_chrdev;
>   
> -	kfd_class = class_create(kfd_dev_name);
> -	err = PTR_ERR(kfd_class);
> -	if (IS_ERR(kfd_class))
> +	err = class_register(&kfd_class);
> +	if (err)
>   		goto err_class_create;
>   
> -	kfd_device = device_create(kfd_class, NULL,
> -					MKDEV(kfd_char_dev_major, 0),
> -					NULL, kfd_dev_name);
> +	kfd_device = device_create(&kfd_class, NULL,
> +				   MKDEV(kfd_char_dev_major, 0),
> +				   NULL, kfd_dev_name);
>   	err = PTR_ERR(kfd_device);
>   	if (IS_ERR(kfd_device))
>   		goto err_device_create;
> @@ -109,7 +110,7 @@ int kfd_chardev_init(void)
>   	return 0;
>   
>   err_device_create:
> -	class_destroy(kfd_class);
> +	class_unregister(&kfd_class);
>   err_class_create:
>   	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
>   err_register_chrdev:
> @@ -118,8 +119,8 @@ int kfd_chardev_init(void)
>   
>   void kfd_chardev_exit(void)
>   {
> -	device_destroy(kfd_class, MKDEV(kfd_char_dev_major, 0));
> -	class_destroy(kfd_class);
> +	device_destroy(&kfd_class, MKDEV(kfd_char_dev_major, 0));
> +	class_unregister(&kfd_class);
>   	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
>   	kfd_device = NULL;
>   }
>
> ---
> base-commit: 8bc75586ea01f1c645063d3472c115ecab03e76c
> change-id: 20240305-class_cleanup-drm-amd-bdc7255b7540
>
> Best regards,

Re: [PATCH] drm/amdkfd: make kfd_class constant

Posted by Ricardo B. Marliere 1 year, 9 months ago

On  5 Mar 12:38, Felix Kuehling wrote:
> On 2024-03-05 7:15, 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 kfd_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>
> 
> The patch looks good to me. Do you want me to apply this to Alex's 
> amd-staging-drm-next?
> 
> Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>

Hi Felix!

Thanks for reviewing. The patch was based in agd5f/linux/drm-next. But I
think amd-staging-drm-next would be fine, too.

Best regards,
-	Ricardo.


> 
> 
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > index f030cafc5a0a..dfa8c69532d4 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> > @@ -63,8 +63,10 @@ static const struct file_operations kfd_fops = {
> >   };
> >   
> >   static int kfd_char_dev_major = -1;
> > -static struct class *kfd_class;
> >   struct device *kfd_device;
> > +static const struct class kfd_class = {
> > +	.name = kfd_dev_name,
> > +};
> >   
> >   static inline struct kfd_process_device *kfd_lock_pdd_by_id(struct kfd_process *p, __u32 gpu_id)
> >   {
> > @@ -94,14 +96,13 @@ int kfd_chardev_init(void)
> >   	if (err < 0)
> >   		goto err_register_chrdev;
> >   
> > -	kfd_class = class_create(kfd_dev_name);
> > -	err = PTR_ERR(kfd_class);
> > -	if (IS_ERR(kfd_class))
> > +	err = class_register(&kfd_class);
> > +	if (err)
> >   		goto err_class_create;
> >   
> > -	kfd_device = device_create(kfd_class, NULL,
> > -					MKDEV(kfd_char_dev_major, 0),
> > -					NULL, kfd_dev_name);
> > +	kfd_device = device_create(&kfd_class, NULL,
> > +				   MKDEV(kfd_char_dev_major, 0),
> > +				   NULL, kfd_dev_name);
> >   	err = PTR_ERR(kfd_device);
> >   	if (IS_ERR(kfd_device))
> >   		goto err_device_create;
> > @@ -109,7 +110,7 @@ int kfd_chardev_init(void)
> >   	return 0;
> >   
> >   err_device_create:
> > -	class_destroy(kfd_class);
> > +	class_unregister(&kfd_class);
> >   err_class_create:
> >   	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
> >   err_register_chrdev:
> > @@ -118,8 +119,8 @@ int kfd_chardev_init(void)
> >   
> >   void kfd_chardev_exit(void)
> >   {
> > -	device_destroy(kfd_class, MKDEV(kfd_char_dev_major, 0));
> > -	class_destroy(kfd_class);
> > +	device_destroy(&kfd_class, MKDEV(kfd_char_dev_major, 0));
> > +	class_unregister(&kfd_class);
> >   	unregister_chrdev(kfd_char_dev_major, kfd_dev_name);
> >   	kfd_device = NULL;
> >   }
> >
> > ---
> > base-commit: 8bc75586ea01f1c645063d3472c115ecab03e76c
> > change-id: 20240305-class_cleanup-drm-amd-bdc7255b7540
> >
> > Best regards,