| Message ID | 2023100654-pointless-stem-5ee1@gregkh (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/habanalabs: make hl_class constant | expand |
On Fri, Oct 6, 2023 at 4:57 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > Now that the driver core allows for struct class to be in read-only > memory, we should make all 'class' structures declared at build time > placing them into read-only memory, instead of having to be dynamically > allocated at runtime. > > This requires some passing of const struct class * around in the common > habanalabs code as well as converting the structure itself. Hi Greg, Thanks for the patch but if you look at our tip of habanalabs-next branch (to be merged in 6.7), you will see the hl_class related code no longer exists, as we moved completely to the new accel char device class. So, I'm dropping this patch. Oded > > Cc: Dafna Hirschfeld <dhirschfeld@habana.ai> > Cc: Dani Liberman <dliberman@habana.ai> > Cc: Koby Elbaz <kelbaz@habana.ai> > Cc: Oded Gabbay <ogabbay@kernel.org> > Cc: Ofir Bitton <obitton@habana.ai> > Cc: Ohad Sharabi <osharabi@habana.ai> > Cc: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > Cc: Tal Cohen <talcohen@habana.ai> > Cc: Tomer Tayar <ttayar@habana.ai> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/accel/habanalabs/common/device.c | 2 +- > drivers/accel/habanalabs/common/habanalabs.h | 2 +- > .../accel/habanalabs/common/habanalabs_drv.c | 17 ++++++++++------- > 3 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c > index b97339d1f7c6..4c28d8cfbb68 100644 > --- a/drivers/accel/habanalabs/common/device.c > +++ b/drivers/accel/habanalabs/common/device.c > @@ -652,7 +652,7 @@ static void device_release_func(struct device *dev) > * > * Initialize a cdev and a Linux device for habanalabs's device. > */ > -static int device_init_cdev(struct hl_device *hdev, struct class *class, > +static int device_init_cdev(struct hl_device *hdev, const struct class *class, > int minor, const struct file_operations *fops, > char *name, struct cdev *cdev, > struct device **dev) > diff --git a/drivers/accel/habanalabs/common/habanalabs.h b/drivers/accel/habanalabs/common/habanalabs.h > index 2f027d5a8206..f1c78555e611 100644 > --- a/drivers/accel/habanalabs/common/habanalabs.h > +++ b/drivers/accel/habanalabs/common/habanalabs.h > @@ -3308,7 +3308,7 @@ struct hl_device { > u64 pcie_bar_phys[HL_PCI_NUM_BARS]; > void __iomem *pcie_bar[HL_PCI_NUM_BARS]; > void __iomem *rmmio; > - struct class *hclass; > + const struct class *hclass; > struct cdev cdev; > struct cdev cdev_ctrl; > struct device *dev; > diff --git a/drivers/accel/habanalabs/common/habanalabs_drv.c b/drivers/accel/habanalabs/common/habanalabs_drv.c > index 7263e84c1a4d..4f1fdff3843d 100644 > --- a/drivers/accel/habanalabs/common/habanalabs_drv.c > +++ b/drivers/accel/habanalabs/common/habanalabs_drv.c > @@ -27,7 +27,11 @@ MODULE_DESCRIPTION(HL_DRIVER_DESC); > MODULE_LICENSE("GPL v2"); > > static int hl_major; > -static struct class *hl_class; > + > +static const struct class hl_class = { > + .name = HL_NAME, > +}; > + > static DEFINE_IDR(hl_devs_idr); > static DEFINE_MUTEX(hl_devs_idr_lock); > > @@ -317,7 +321,7 @@ static void copy_kernel_module_params_to_device(struct hl_device *hdev) > hdev->asic_prop.fw_security_enabled = is_asic_secured(hdev->asic_type); > > hdev->major = hl_major; > - hdev->hclass = hl_class; > + hdev->hclass = &hl_class; > hdev->memory_scrub = memory_scrub; > hdev->reset_on_lockup = reset_on_lockup; > hdev->boot_error_status_mask = boot_error_status_mask; > @@ -691,10 +695,9 @@ static int __init hl_init(void) > > hl_major = MAJOR(dev); > > - hl_class = class_create(HL_NAME); > - if (IS_ERR(hl_class)) { > + rc = class_register(&hl_class); > + if (rc) { > pr_err("failed to allocate class\n"); > - rc = PTR_ERR(hl_class); > goto remove_major; > } > > @@ -712,7 +715,7 @@ static int __init hl_init(void) > > remove_debugfs: > hl_debugfs_fini(); > - class_destroy(hl_class); > + class_unregister(&hl_class); > remove_major: > unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS); > return rc; > @@ -732,7 +735,7 @@ static void __exit hl_exit(void) > */ > hl_debugfs_fini(); > > - class_destroy(hl_class); > + class_unregister(&hl_class); > unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS); > > idr_destroy(&hl_devs_idr); > -- > 2.42.0 >
On Tue, Oct 10, 2023 at 09:07:53AM +0300, Oded Gabbay wrote: > On Fri, Oct 6, 2023 at 4:57 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > Now that the driver core allows for struct class to be in read-only > > memory, we should make all 'class' structures declared at build time > > placing them into read-only memory, instead of having to be dynamically > > allocated at runtime. > > > > This requires some passing of const struct class * around in the common > > habanalabs code as well as converting the structure itself. > > Hi Greg, > Thanks for the patch but if you look at our tip of habanalabs-next > branch (to be merged in 6.7), you will see the hl_class related code > no longer exists, as we moved completely to the new accel char device > class. > So, I'm dropping this patch. Ah, no code is better, thanks! greg k-h
diff --git a/drivers/accel/habanalabs/common/device.c b/drivers/accel/habanalabs/common/device.c index b97339d1f7c6..4c28d8cfbb68 100644 --- a/drivers/accel/habanalabs/common/device.c +++ b/drivers/accel/habanalabs/common/device.c @@ -652,7 +652,7 @@ static void device_release_func(struct device *dev) * * Initialize a cdev and a Linux device for habanalabs's device. */ -static int device_init_cdev(struct hl_device *hdev, struct class *class, +static int device_init_cdev(struct hl_device *hdev, const struct class *class, int minor, const struct file_operations *fops, char *name, struct cdev *cdev, struct device **dev) diff --git a/drivers/accel/habanalabs/common/habanalabs.h b/drivers/accel/habanalabs/common/habanalabs.h index 2f027d5a8206..f1c78555e611 100644 --- a/drivers/accel/habanalabs/common/habanalabs.h +++ b/drivers/accel/habanalabs/common/habanalabs.h @@ -3308,7 +3308,7 @@ struct hl_device { u64 pcie_bar_phys[HL_PCI_NUM_BARS]; void __iomem *pcie_bar[HL_PCI_NUM_BARS]; void __iomem *rmmio; - struct class *hclass; + const struct class *hclass; struct cdev cdev; struct cdev cdev_ctrl; struct device *dev; diff --git a/drivers/accel/habanalabs/common/habanalabs_drv.c b/drivers/accel/habanalabs/common/habanalabs_drv.c index 7263e84c1a4d..4f1fdff3843d 100644 --- a/drivers/accel/habanalabs/common/habanalabs_drv.c +++ b/drivers/accel/habanalabs/common/habanalabs_drv.c @@ -27,7 +27,11 @@ MODULE_DESCRIPTION(HL_DRIVER_DESC); MODULE_LICENSE("GPL v2"); static int hl_major; -static struct class *hl_class; + +static const struct class hl_class = { + .name = HL_NAME, +}; + static DEFINE_IDR(hl_devs_idr); static DEFINE_MUTEX(hl_devs_idr_lock); @@ -317,7 +321,7 @@ static void copy_kernel_module_params_to_device(struct hl_device *hdev) hdev->asic_prop.fw_security_enabled = is_asic_secured(hdev->asic_type); hdev->major = hl_major; - hdev->hclass = hl_class; + hdev->hclass = &hl_class; hdev->memory_scrub = memory_scrub; hdev->reset_on_lockup = reset_on_lockup; hdev->boot_error_status_mask = boot_error_status_mask; @@ -691,10 +695,9 @@ static int __init hl_init(void) hl_major = MAJOR(dev); - hl_class = class_create(HL_NAME); - if (IS_ERR(hl_class)) { + rc = class_register(&hl_class); + if (rc) { pr_err("failed to allocate class\n"); - rc = PTR_ERR(hl_class); goto remove_major; } @@ -712,7 +715,7 @@ static int __init hl_init(void) remove_debugfs: hl_debugfs_fini(); - class_destroy(hl_class); + class_unregister(&hl_class); remove_major: unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS); return rc; @@ -732,7 +735,7 @@ static void __exit hl_exit(void) */ hl_debugfs_fini(); - class_destroy(hl_class); + class_unregister(&hl_class); unregister_chrdev_region(MKDEV(hl_major, 0), HL_MAX_MINORS); idr_destroy(&hl_devs_idr);
Now that the driver core allows for struct class to be in read-only memory, we should make all 'class' structures declared at build time placing them into read-only memory, instead of having to be dynamically allocated at runtime. This requires some passing of const struct class * around in the common habanalabs code as well as converting the structure itself. Cc: Dafna Hirschfeld <dhirschfeld@habana.ai> Cc: Dani Liberman <dliberman@habana.ai> Cc: Koby Elbaz <kelbaz@habana.ai> Cc: Oded Gabbay <ogabbay@kernel.org> Cc: Ofir Bitton <obitton@habana.ai> Cc: Ohad Sharabi <osharabi@habana.ai> Cc: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> Cc: Tal Cohen <talcohen@habana.ai> Cc: Tomer Tayar <ttayar@habana.ai> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/accel/habanalabs/common/device.c | 2 +- drivers/accel/habanalabs/common/habanalabs.h | 2 +- .../accel/habanalabs/common/habanalabs_drv.c | 17 ++++++++++------- 3 files changed, 12 insertions(+), 9 deletions(-)