search for: ida_destroy

Hi, The following patches fix couple of memory leaks in the virtio and remoteproc cores when using these as modules, and going through a cycle of insmod and rmmod with at least a device registered with the corresponding cores in between. I ran into this on our downstream product kernels on both 3.14 and 4.1 based kernels, and should apply to the latest kernel as well. Patches can be picked up

Hi, The following patches fix couple of memory leaks in the virtio and remoteproc cores when using these as modules, and going through a cycle of insmod and rmmod with at least a device registered with the corresponding cores in between. I ran into this on our downstream product kernels on both 3.14 and 4.1 based kernels, and should apply to the latest kernel as well. Patches can be picked up

...n, and all these layers are > truely freed only upon the ida destruction. The virtio_index_ida > is not destroyed at present, leading to a memory leak when using > the virtio core as a module and atleast one virtio device is > registered and unregistered. > > Fix this by invoking ida_destroy() in the virtio core module > exit. > > Cc: "Michael S. Tsirkin" <mst at redhat.com> > Signed-off-by: Suman Anna <s-anna at ti.com> Interesting. Will the same apply to e.g. sd_index_ida in drivers/scsi/sd.c or iscsi_sess_ida in drivers/scsi/scsi_transport_iscsi.c...

...n, and all these layers are > truely freed only upon the ida destruction. The virtio_index_ida > is not destroyed at present, leading to a memory leak when using > the virtio core as a module and atleast one virtio device is > registered and unregistered. > > Fix this by invoking ida_destroy() in the virtio core module > exit. > > Cc: "Michael S. Tsirkin" <mst at redhat.com> > Signed-off-by: Suman Anna <s-anna at ti.com> Interesting. Will the same apply to e.g. sd_index_ida in drivers/scsi/sd.c or iscsi_sess_ida in drivers/scsi/scsi_transport_iscsi.c...

...upon the ida destruction. The virtio_index_ida > > > is not destroyed at present, leading to a memory leak when using > > > the virtio core as a module and atleast one virtio device is > > > registered and unregistered. > > > > > > Fix this by invoking ida_destroy() in the virtio core module > > > exit. > > > > > > Cc: "Michael S. Tsirkin" <mst at redhat.com> > > > Signed-off-by: Suman Anna <s-anna at ti.com> > > > > Interesting. > > Will the same apply to e.g. sd_index_ida in driv...

...p upon any ida allocation, and all these layers are truely freed only upon the ida destruction. The rproc_dev_index ida is not destroyed at present, leading to a memory leak when using the remoteproc core as a module and atleast one rproc device is registered and unregistered. Fix this by invoking ida_destroy() in the remoteproc core module exit. Cc: Ohad Ben-Cohen <ohad at wizery.com> Signed-off-by: Suman Anna <s-anna at ti.com> --- drivers/remoteproc/remoteproc_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remotepro...

...bitmap upon any ida allocation, and all these layers are truely freed only upon the ida destruction. The virtio_index_ida is not destroyed at present, leading to a memory leak when using the virtio core as a module and atleast one virtio device is registered and unregistered. Fix this by invoking ida_destroy() in the virtio core module exit. Cc: "Michael S. Tsirkin" <mst at redhat.com> Signed-off-by: Suman Anna <s-anna at ti.com> --- drivers/virtio/virtio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index b1877d73fa56..7...

...are >> truely freed only upon the ida destruction. The virtio_index_ida >> is not destroyed at present, leading to a memory leak when using >> the virtio core as a module and atleast one virtio device is >> registered and unregistered. >> >> Fix this by invoking ida_destroy() in the virtio core module >> exit. >> >> Cc: "Michael S. Tsirkin" <mst at redhat.com> >> Signed-off-by: Suman Anna <s-anna at ti.com> > > Interesting. > Will the same apply to e.g. sd_index_ida in drivers/scsi/sd.c > or iscsi_sess_ida in...

...d no different > from a dynamically allocated struct ida. Or are you thinking because > ida_destory() doesn't set bitmap to NULL, it damages the reuse? In > which case I'm not sure there's much benefit to making it reusable, but > I suppose we could by adding a memset into ida_destroy(). I don't know. Data structures which do lazy anything would likely need explicit destruction and I'm not sure we'd wanna ban static initialization for all such cases. Seems like an unnecessary restriction. Thanks. -- tejun

...t. We could have ida_remove() actually > free the bitmap and not cache it if it's the last layer. That way ida > would naturally empty and we wouldn't need a destructor. Tejun, would > that work? Yeah, that definitely is one way to go about it. It kinda muddles the purpose of ida_destroy() tho. I suppose we can rename it to idr_remove_all() and then do the same to idr. I'm not particularly objecting to all that but what's wrong with just calling idr_destroy() on exit paths? If missing the call in modules is an issue, maybe we can just annotate idr/ida with debugobj? Tha...

...s that if we can't follow the no destructor pattern for > DEFINE_IDA() then we shouldn't have it at all, because it's confusing > kernel design patterns. The pattern we would have would be > > struct ida someida: > > ida_init(&someida); > > ... > > ida_destroy(&someida); > > so the object explicitly has a constructor matched to a destructor. Yeah, I get that. I'm just not convinced that this matters enough especially if we can get debugobj/ksan/whatever trip on it. Thanks. -- tejun

...t;id >= 0) + ida_simple_remove(&bl_ida, connector->id); + } if (drm->backlight) { backlight_device_unregister(drm->backlight); drm->backlight = NULL; } } + +void +nouveau_backlight_ctor(void) +{ + ida_init(&bl_ida); +} + +void +nouveau_backlight_dtor(void) +{ + ida_destroy(&bl_ida); +} diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index 330fe0f..4a75df0 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -91,6 +91,8 @@ int nouveau_crtc_set_config(struct drm_...

Currently, every backlight interface created by Nouveau uses the same name, nv_backlight. This leads to a sysfs warning as it tries to create an already existing folder. This patch adds a incremented number to the name, but keeps the initial name as nv_backlight, to avoid possibly breaking userspace; the second interface will be named nv_backlight1, and so on. Fixes: fdo#86539 Signed-off-by:

...t;id >= 0) + ida_simple_remove(&bl_ida, connector->id); + } if (drm->backlight) { backlight_device_unregister(drm->backlight); drm->backlight = NULL; } } + +void +nouveau_backlight_ctor(void) +{ + ida_init(&bl_ida); +} + +void +nouveau_backlight_dtor(void) +{ + ida_destroy(&bl_ida); +} diff --git a/drm/nouveau/nouveau_display.h b/drm/nouveau/nouveau_display.h index 5a57d8b..bb07cc8 100644 --- a/drm/nouveau/nouveau_display.h +++ b/drm/nouveau/nouveau_display.h @@ -90,6 +90,8 @@ int nouveau_crtc_set_config(struct drm_mode_set *set); #ifdef CONFIG_DRM_NOUVEAU_BACKL...

...t;id >= 0) + ida_simple_remove(&bl_ida, connector->id); + } if (drm->backlight) { backlight_device_unregister(drm->backlight); drm->backlight = NULL; } } + +void +nouveau_backlight_ctor(void) +{ + ida_init(&bl_ida); +} + +void +nouveau_backlight_dtor(void) +{ + ida_destroy(&bl_ida); +} diff --git a/drm/nouveau/nouveau_display.h b/drm/nouveau/nouveau_display.h index 330fe0f..4a75df0 100644 --- a/drm/nouveau/nouveau_display.h +++ b/drm/nouveau/nouveau_display.h @@ -91,6 +91,8 @@ int nouveau_crtc_set_config(struct drm_mode_set *set); #ifdef CONFIG_DRM_NOUVEAU_BACKL...

...if (drm->backlight) { > backlight_device_unregister(drm->backlight); > drm->backlight = NULL; > } > } > + > +void > +nouveau_backlight_ctor(void) > +{ > + ida_init(&bl_ida); > +} > + > +void > +nouveau_backlight_dtor(void) > +{ > + ida_destroy(&bl_ida); > +} > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h > index 330fe0f..4a75df0 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.h > +++ b/drivers/gpu/drm/nouveau/nouveau_display.h > @@ -91,6 +91,8 @@ int nouve...

...r_unregister(&drv->driver); +} +EXPORT_SYMBOL_GPL(vdpa_unregister_driver); + +static int vdpa_init(void) +{ + if (bus_register(&vdpa_bus) != 0) + panic("virtio bus registration failed"); + return 0; +} + +static void __exit vdpa_exit(void) +{ + bus_unregister(&vdpa_bus); + ida_destroy(&vdpa_index_ida); +} +core_initcall(vdpa_init); +module_exit(vdpa_exit); + +MODULE_AUTHOR("Jason Wang <jasowang at redhat.com>"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h new file mode 100644 index 000000000000..52543a862801 -...

...*/ +void vdpa_unregister_driver(struct vdpa_driver *drv) +{ + driver_unregister(&drv->driver); +} +EXPORT_SYMBOL_GPL(vdpa_unregister_driver); + +static int vdpa_init(void) +{ + return bus_register(&vdpa_bus); +} + +static void __exit vdpa_exit(void) +{ + bus_unregister(&vdpa_bus); + ida_destroy(&vdpa_index_ida); +} +core_initcall(vdpa_init); +module_exit(vdpa_exit); + +MODULE_AUTHOR("Jason Wang <jasowang at redhat.com>"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h new file mode 100644 index 000000000000..7cb8ef32e2e8 -...

...ailed ret %d\n", ret); + goto class_destroy; + } + return ret; +class_destroy: + class_destroy(g_mic_class); +cleanup_chrdev: + unregister_chrdev_region(g_mic_devno, MIC_MAX_NUM_DEVS); +error: + return ret; +} + +static void __exit mic_exit(void) +{ + pci_unregister_driver(&mic_driver); + ida_destroy(&g_mic_ida); + class_destroy(g_mic_class); + unregister_chrdev_region(g_mic_devno, MIC_MAX_NUM_DEVS); +} + +module_init(mic_init); +module_exit(mic_exit); + +MODULE_AUTHOR("Intel Corporation"); +MODULE_DESCRIPTION("Intel(R) MIC X100 Host driver"); +MODULE_LICENSE("GPL v...

Next version of https://patchwork.freedesktop.org/series/48596/ . Made some important changes to the refactoring patch, but everything else is the same. Lyude Paul (5): drm/nouveau: Check backlight IDs are >= 0, not > 0 drm/nouveau: Move backlight device into nouveau_connector drm/nouveau: s/nouveau_backlight_exit/nouveau_backlight_fini/ drm/nouveau: Cleanup indenting in