Jan Kiszka <jan.kis...@siemens.com> writes:
> On 2012-08-27 17:14, Anthony Liguori wrote:
>> Jan Kiszka <jan.kis...@siemens.com> writes:
>>
>>> On 2012-08-27 15:19, Anthony Liguori wrote:
>>>> Liu Ping Fan <qemul...@gmail.com> writes:
>>>>
>>>>> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>>>>>
>>>>> Scene:
>>>>> obja lies in objA, when objA's ref->0, it will be freed,
>>>>> but at that time obja can still be in use.
>>>>>
>>>>> The real example is:
>>>>> typedef struct PCIIDEState {
>>>>> PCIDevice dev;
>>>>> IDEBus bus[2]; --> create in place
>>>>> .....
>>>>> }
>>>>>
>>>>> When without big lock protection for mmio-dispatch, we will hold
>>>>> obj's refcnt. So memory_region_init_io() will replace the third para
>>>>> "void *opaque" with "Object *obj".
>>>>> With this patch, we can protect PCIIDEState from disappearing during
>>>>> mmio-dispatch hold the IDEBus->ref.
>>>>>
>>>>> And the ref circle has been broken when calling qdev_delete_subtree().
>>>>>
>>>>> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
>>>>
>>>> I think this is solving the wrong problem. There are many, many
>>>> dependencies a device may have on other devices. Memory allocation
>>>> isn't the only one.
>>>>
>>>> The problem is that we want to make sure that a device doesn't "go away"
>>>> while an MMIO dispatch is happening. This is easy to solve without
>>>> touching referencing counting.
>>>>
>>>> The device will hold a lock while the MMIO is being dispatched. The
>>>> delete path simply needs to acquire that same lock. This will ensure
>>>> that a delete operation cannot finish while MMIO is still in flight.
>>>
>>> That's a bit too simple. Quite a few MMIO/PIO fast-paths will work
>>> without any device-specific locking, e.g. just to read a simple register
>>> value. So we will need reference counting
>>
>> But then you'll need to acquire a lock to take the reference/remove the
>> reference which sort of defeats the purpose of trying to fast path.
>
> Atomic ops? RCU? This problem won't be solved for the first time.
Yes, there are ways to do this, but you add a fair bit of complication.
It's much simplier to make objects not have any locks and enforce all
callers to protect access. IOW, noone except the device gets to inc/dec
reference counts.
>>> (for devices using private
>>> locks), but on the "front-line" object: the memory region. That region
>>> will block its owner from disappearing by waiting on dispatch when
>>> someone tries to unregister it.
>>>
>>> Also note that "holding a lock" is easily said but will be more tricky
>>> in practice. Quite a significant share of our code will continue to run
>>> under BQL, even for devices with their own locks. Init/cleanup functions
>>> will likely fall into this category,
>>
>> I'm not sure I'm convinced of this--but it's hard to tell until we
>> really start converting.
>>
>> BTW, I'm pretty sure we have to tackle main loop functions first before
>> we try to convert any devices off the BQL.
>
> I'm sure we should leave existing code alone wherever possible, focusing
> on providing alternative versions for those paths that matter. Example:
> Most timers are fine under BQL. But some sensitive devices (RTC or HPET
> as clock source) will want their own timers. So the approach is to
> instantiate a separate, also prioritizeable instance of the timer
> subsystem for them and be done.
I disagree. I think we conver the timer subsystem to be lockless and
then let some devices acquire the BQL during dispatch.
And we have a nice thread-aware main loop available to us--glib. We
don't need to reinvent the wheel.
Regards,
Anthony Liguori
>
> We won't convert QEMU in a day, but we surely want results before the
> last corner is refactored (which would take years, at best).
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
