qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Joao Martins <joao.m.martins@oracle.com>
To: "Duan, Zhenzhong" <zhenzhong.duan@intel.com>
Cc: "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"clg@redhat.com" <clg@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"avihaih@nvidia.com" <avihaih@nvidia.com>,
	"Peng, Chao P" <chao.p.peng@intel.com>
Subject: Re: [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize
Date: Tue, 27 Jun 2023 11:21:36 +0100	[thread overview]
Message-ID: <8ef7f136-54ce-16dd-60fd-fb360b012646@oracle.com> (raw)
In-Reply-To: <SJ0PR11MB6744DB25CF81FF634B82FF7A9227A@SJ0PR11MB6744.namprd11.prod.outlook.com>

>>>  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.


  reply	other threads:[~2023-06-27 10:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-21  8:02 [PATCH v3 0/3] VFIO migration related refactor and bug fix Zhenzhong Duan
2023-06-21  8:02 ` [PATCH v3 1/3] vfio/pci: Fix resource leak in vfio_realize Zhenzhong Duan
2023-06-21 11:08   ` Joao Martins
2023-06-25  6:00     ` Duan, Zhenzhong
2023-06-26  7:02       ` Duan, Zhenzhong
2023-06-26 10:07         ` Joao Martins
2023-06-27  2:38           ` Duan, Zhenzhong
2023-06-27 10:21             ` Joao Martins [this message]
2023-06-27 10:28               ` Duan, Zhenzhong
2023-06-21  8:02 ` [PATCH v3 2/3] vfio/pci: Fix a segfault " Zhenzhong Duan
2023-06-21 11:08   ` Joao Martins
2023-06-25  6:01     ` Duan, Zhenzhong
2023-06-26  9:58       ` Joao Martins
2023-06-21  8:02 ` [PATCH v3 3/3] vfio/migration: vfio/migration: Refactor and fix print of "Migration disabled" Zhenzhong Duan
2023-06-26  9:34   ` Avihai Horon
2023-06-26 10:18     ` Joao Martins
2023-06-27  2:55       ` Duan, Zhenzhong
2023-06-27 10:56         ` Joao Martins
2023-06-28  2:26           ` Duan, Zhenzhong
2023-06-27  2:46     ` Duan, Zhenzhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ef7f136-54ce-16dd-60fd-fb360b012646@oracle.com \
    --to=joao.m.martins@oracle.com \
    --cc=alex.williamson@redhat.com \
    --cc=avihaih@nvidia.com \
    --cc=chao.p.peng@intel.com \
    --cc=clg@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zhenzhong.duan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).