Messages in this thread | | | Date | Thu, 17 Sep 2015 08:33:33 +0300 | From | "Michael S. Tsirkin" <> | Subject | DEFINE_IDA causing memory leaks? (was Re: [PATCH 1/2] virtio: fix memory leak of virtio ida cache layers) |
| |
On Wed, Sep 16, 2015 at 07:29:17PM -0500, Suman Anna wrote: > The virtio core uses a static ida named virtio_index_ida for > assigning index numbers to virtio devices during registration. > The ida core may allocate some internal idr cache layers and > an ida 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@redhat.com> > Signed-off-by: Suman Anna <s-anna@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?
If no, why not?
One doesn't generally expect to have to free global variables. Maybe we should forbid DEFINE_IDA in modules?
James, could you comment on this please?
> --- > drivers/virtio/virtio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > index b1877d73fa56..7062bb0975a5 100644 > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -412,6 +412,7 @@ static int virtio_init(void) > static void __exit virtio_exit(void) > { > bus_unregister(&virtio_bus); > + ida_destroy(&virtio_index_ida); > } > core_initcall(virtio_init); > module_exit(virtio_exit); > -- > 2.5.0
| |