>-----Original Message----- >From: Joao Martins <joao.m.mart...@oracle.com> >Sent: Tuesday, June 27, 2023 6:22 PM >To: Duan, Zhenzhong <zhenzhong.d...@intel.com> >Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >avih...@nvidia.com; Peng, Chao P <chao.p.p...@intel.com> >Subject: Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize > >>>> out_deregister: >>>> pci_device_set_intx_routing_notifier(&vdev->pdev, NULL); >>>> if (vdev->irqchip_change_notifier.notify) { >>>> kvm_irqchip_remove_change_notifier(&vdev- >>irqchip_change_notifier); >>>> } >>>> + vfio_disable_interrupts(vdev); >>>> + if (vdev->intx.mmap_timer) { >>>> + timer_free(vdev->intx.mmap_timer); >>>> + } >>> >>> But this one suggests another one as it looks a pre-existing issue? >> Yes, it's another resource leak I just found. >> Not sure if it's better to also merge above change to this patch which >> is targeting resource leak issues, or to PATCH2 which is targeting >out_deregister path, or to create a new one. >> Any suggestion? > >In general they are all bugs in the same deregistration path, but each resource >is not being teardown correctly. I tend to prefer 'logical change' per commit, >so there's would be a fix the irqchip_change notifier and another one for >mmap_timer teardown. Both with the Fixes tags that introduced each bug. >Unless everything was introduced by the same change in which case you >would do everything in one patch. Clear.
Thanks Zhenzhong