[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 0/4] Plug some memory leaks on unrealize
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 0/4] Plug some memory leaks on unrealize |
Date: |
Sat, 23 Jul 2016 17:16:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 23/07/2016 15:18, Corey Minyard wrote:
> On 07/23/2016 02:46 AM, Paolo Bonzini wrote:
>>
>> On 22/07/2016 21:50, address@hidden wrote:
>>> This has kind of opened a can of worms for me, though. Looking
>>> at a lot of the devices, there is no unrealize function and that
>>> can leave a lot of things hanging. And for ISA bus devices, there
>>> is no way to unregister ports.
>> Right, this is because they aren't hotpluggable.
>>
>> I should dig out the huge patchset I had to make timers statically
>> allocated...
>>
>> Paolo
> Am I correct in saying, then, that instead of adding a finalize
> function to the IPMI BMC, we should instead make it not hot
> pluggable? And then the rest of my patches are not really
> relevant. I already have a function to set hotpluggable to
> false for the BMCs, I can post that.
If they are ISA devices they should already not be hot-unpluggable,
because none of the ISA bridges implements HotplugHandler. Because
that's just the way the bus works, it shouldn't be an issue.
> From what I have seen, you can unrealize devices using the
> API, even if they are not hot pluggable, by setting the realized
> bool. Is that ok?
It's not great, but it's not a big deal either.
The original idea behind "realize" was to have it as a sort of Vcc pin
where a false/true pulse would work as a reset, but this never
materialized. Now the true->false transition on realize is really only
used as part of a full guest-triggered hot-unplug sequence, which is
guest->hotplug_handler_unplug->(method call)->object_unparent.
Because all HotplugHandlers call object_unparent, which in turn ends up
freeing the object, a false->true->false transition on realized (and
thus the timer leak) is not guest-triggerable.
There are various fixes, including:
- making the device non-hotpluggable
- moving the timer_new and timer_free respectively to instance_init and
instance_finalize
- making the timer static, which requires some small changes in the
timer API. Most of the last bullet is scriptable with Coccinelle.
Right now I'd just do #2 or don't bother.
Paolo
- [Qemu-devel] [PATCH v3 0/4] Plug some memory leaks on unrealize, minyard, 2016/07/22
- [Qemu-devel] [PATCH v3 2/4] wdt_i6300esb: Free timer, minyard, 2016/07/22
- [Qemu-devel] [PATCH v3 3/4] wdt_ib700: Free timer, minyard, 2016/07/22
- [Qemu-devel] [PATCH v3 1/4] ipmi_bmc_sim: Remove an unnecessary mutex, minyard, 2016/07/22
- [Qemu-devel] [PATCH v3 4/4] ipmi_bmc_sim: Add a proper unrealize function, minyard, 2016/07/22
- Re: [Qemu-devel] [PATCH v3 0/4] Plug some memory leaks on unrealize, Paolo Bonzini, 2016/07/23