On 09/17/2015 07:33 AM, Michael S. Tsirkin wrote: > 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@xxxxxxxxxx> >> Signed-off-by: Suman Anna <s-anna@xxxxxx> > > 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? > Well, looking at the code 'ida_destroy' only need to be called if you want/need to do a general cleanup. It shouldn't be required if you do correct reference counting on your objects, and call idr_remove() on each of them. Unless I'm misreading something. Seems like a topic for KS; Johannes had a larger patchset recently to clean up idr, which run into very much the same issues. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
