| Message ID | 20200327095642.18424-3-pannengyuan@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix two virtio queues memleak | expand |
On Fri, Mar 27, 2020 at 05:56:42PM +0800, Pan Nengyuan wrote: > req_vq/event_vq forgot to free in unrealize. Fix that. > And aslo do clean 's->as_by_busptr' hash table in unrealize to fix another leak. s/aslo/also Maybe we can also update the subject in something like this: "virtio-iommu: avoid memleak in the unrealize" > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > Acked-by: Eric Auger <eric.auger@redhat.com> > --- > Cc: Eric Auger <eric.auger@redhat.com> > --- > v3->v1/v2: > - Aslo clean 's->as_by_busptr' hash table in unrealize.(Suggested by Stefano Garzarella) > --- > hw/virtio/virtio-iommu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > index 4cee8083bc..694698fa0f 100644 > --- a/hw/virtio/virtio-iommu.c > +++ b/hw/virtio/virtio-iommu.c > @@ -696,7 +696,10 @@ static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp) > g_tree_destroy(s->domains); > g_tree_destroy(s->endpoints); > > + virtio_delete_queue(s->req_vq); > + virtio_delete_queue(s->event_vq); > virtio_cleanup(vdev); > + g_hash_table_destroy(s->as_by_busptr); If you need to respin, you could move g_hash_table_destroy() at the beggining of unrealize(), just to follow a reverse order of realize(). Thanks, Stefano > } > > static void virtio_iommu_device_reset(VirtIODevice *vdev) > -- > 2.18.2 > >
On 3/28/2020 12:26 AM, Stefano Garzarella wrote: > On Fri, Mar 27, 2020 at 05:56:42PM +0800, Pan Nengyuan wrote: >> req_vq/event_vq forgot to free in unrealize. Fix that. >> And aslo do clean 's->as_by_busptr' hash table in unrealize to fix another leak. > > s/aslo/also > > Maybe we can also update the subject in something like this: > "virtio-iommu: avoid memleak in the unrealize" OK. > >> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> >> Acked-by: Eric Auger <eric.auger@redhat.com> >> --- >> Cc: Eric Auger <eric.auger@redhat.com> >> --- >> v3->v1/v2: >> - Aslo clean 's->as_by_busptr' hash table in unrealize.(Suggested by Stefano Garzarella) >> --- >> hw/virtio/virtio-iommu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c >> index 4cee8083bc..694698fa0f 100644 >> --- a/hw/virtio/virtio-iommu.c >> +++ b/hw/virtio/virtio-iommu.c >> @@ -696,7 +696,10 @@ static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp) >> g_tree_destroy(s->domains); >> g_tree_destroy(s->endpoints); >> >> + virtio_delete_queue(s->req_vq); >> + virtio_delete_queue(s->event_vq); >> virtio_cleanup(vdev); >> + g_hash_table_destroy(s->as_by_busptr); > > If you need to respin, you could move g_hash_table_destroy() > at the beggining of unrealize(), just to follow a reverse order of > realize(). OK. Thanks. > > Thanks, > Stefano > >> } >> >> static void virtio_iommu_device_reset(VirtIODevice *vdev) >> -- >> 2.18.2 >> >> >
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 4cee8083bc..694698fa0f 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -696,7 +696,10 @@ static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp) g_tree_destroy(s->domains); g_tree_destroy(s->endpoints); + virtio_delete_queue(s->req_vq); + virtio_delete_queue(s->event_vq); virtio_cleanup(vdev); + g_hash_table_destroy(s->as_by_busptr); } static void virtio_iommu_device_reset(VirtIODevice *vdev)