[PATCH 03/14] hw/isa/vt82c686: Free irqs

Akihiko Odaki posted 14 patches 5 months ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, John Snow <jsnow@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Alexey Kardashevskiy <aik@ozlabs.ru>, David Gibson <david@gibson.dropbear.id.au>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH 03/14] hw/isa/vt82c686: Free irqs
Posted by Akihiko Odaki 5 months ago
This suppresses LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/isa/vt82c686.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8582ac0322eb..189b487f1d22 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -721,7 +721,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
     qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
-    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
                           errp);
 
@@ -729,7 +728,9 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
         return;
     }
 
+    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
+    qemu_free_irqs(isa_irq, 1);
     isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(OBJECT(d), isa_bus, 0);

-- 
2.45.2
Re: [PATCH 03/14] hw/isa/vt82c686: Free irqs
Posted by Peter Maydell 5 months ago
On Wed, 26 Jun 2024 at 12:08, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> This suppresses LeakSanitizer warnings.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/isa/vt82c686.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 8582ac0322eb..189b487f1d22 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -721,7 +721,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>
>      qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>      qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
> -    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>      isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>                            errp);
>
> @@ -729,7 +728,9 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>          return;
>      }
>
> +    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>      s->isa_irqs_in = i8259_init(isa_bus, *isa_irq);
> +    qemu_free_irqs(isa_irq, 1);

This looks pretty weird -- we allocate the irq array, and
then immediately free it. The memory ought not to be
freed until the end of the simulation.

The ideal way to resolve this kind of leak with qemu_allocate_irqs()
is to avoid using the function entirely, and instead use IRQ
arrays allocated via qdev_init_gpio_* or the sysbus IRQ APIs.
qemu_allocate_irqs() is old and a sort of inherently leaky API.

The less ideal way is to keep the pointer to the array in the
device stuct.

If you look through 'git log' for qemu_allocate_irqs() you'll
see various places in the past where we've refactored things
to avoid this kind of uninteresting memory leak.

>      isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
>      i8254_pit_init(isa_bus, 0x40, 0, NULL);
>      i8257_dma_init(OBJECT(d), isa_bus, 0);

thanks
-- PMM