[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 24/37] pc: keep gsi reference
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH 24/37] pc: keep gsi reference |
Date: |
Thu, 21 Jul 2016 14:28:33 -0400 (EDT) |
Hi
----- Original Message -----
> On Thu, Jul 21, 2016 at 01:27:35PM -0400, Marc-André Lureau wrote:
> > Hi
> >
> > ----- Original Message -----
> > > On Tue, Jul 19, 2016 at 12:54:19PM +0400, address@hidden
> > > wrote:
> > > > From: Marc-André Lureau <address@hidden>
> > > >
> > > > Further cleanup would need to call qemu_free_irq() at the appropriate
> > > > time, but for now this silences ASAN about direct leaks.
> > > >
> > > > Signed-off-by: Marc-André Lureau <address@hidden>
> > >
> > > Is there a way to make ASAN happy without having to add a field
> > > to MachineState that we're not going to use for anything?
> >
> > Well, the plan is rather to release it when no longer needed.
> > Would it be fine to call qemu_free_irqs() in
> > machine_finalize()?
>
> It would be fine, I guess, but it looks pointless if we have lots
> of other resources allocated during PC machine initialization
> that are never released.
The main point, right now, is to have no direct leaks when running ASAN or
valgrind, as they hide new introduced leaks that may be much worse. (it would
also be good if we had no indirect leaks either, as this may also grow over
time)
> But, see additional comment below:
>
> >
> > >
> > > > ---
> > > > hw/i386/pc_piix.c | 1 +
> > > > hw/i386/pc_q35.c | 1 +
> > > > include/hw/boards.h | 1 +
> > > > 3 files changed, 3 insertions(+)
> > > >
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index a07dc81..b2db274 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -190,6 +190,7 @@ static void pc_init1(MachineState *machine,
> > > > } else {
> > > > gsi = qemu_allocate_irqs(gsi_handler, gsi_state,
> > > > GSI_NUM_PINS);
> > > > }
> > > > + machine->gsi = gsi;
> > > >
> > > > if (pcmc->pci_enabled) {
> > > > pci_bus = i440fx_init(host_type,
> > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > > index c5e8367..5dfb14f 100644
> > > > --- a/hw/i386/pc_q35.c
> > > > +++ b/hw/i386/pc_q35.c
> > > > @@ -158,6 +158,7 @@ static void pc_q35_init(MachineState *machine)
> > > > } else {
> > > > gsi = qemu_allocate_irqs(gsi_handler, gsi_state,
> > > > GSI_NUM_PINS);
> > > > }
> > > > + machine->gsi = gsi;
> > > >
> > > > /* create pci host bus */
> > > > q35_host = Q35_HOST_DEVICE(qdev_create(NULL,
> > > > TYPE_Q35_HOST_DEVICE));
> > > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > > index e46a744..289ba52 100644
> > > > --- a/include/hw/boards.h
> > > > +++ b/include/hw/boards.h
> > > > @@ -139,6 +139,7 @@ struct MachineState {
> > > > /*< private >*/
> > > > Object parent_obj;
> > > > Notifier sysbus_notifier;
> > > > + qemu_irq *gsi;
>
> If this is used only by PC, doesn't it belong to PCMachineState?
right, i'll try to put it there
> Anyway, the new field would be very useful to help reduce the
> number of parameters of PC initialization functions (by making
> them just get a PCMachineState* argument). I would go even
Which functions do you have in mind?
> further and remove the local 'gsi' variable and replace it with
> 'pcms->gsi' everywhere.
ok, why not.
- [Qemu-devel] [PATCH 19/37] char: disconnect peer when qemu_chr_free(), (continued)
- [Qemu-devel] [PATCH 19/37] char: disconnect peer when qemu_chr_free(), marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 20/37] char: free MuxDriver when closing, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 21/37] tests: fix qom-test leaks, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 22/37] pc: free i8259, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 23/37] pci-bus: do not allocate and leak bsel, marcandre . lureau, 2016/07/19
- [Qemu-devel] [PATCH 24/37] pc: keep gsi reference, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 25/37] ahci: free irqs array, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 26/37] sd: free timer, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 27/37] qjson: free str, marcandre . lureau, 2016/07/19
[Qemu-devel] [PATCH 28/37] virtio-input: free config list, marcandre . lureau, 2016/07/19