Re: [PATCH 06/15] memory: use refcnt to manage MemoryRegion

[liu ping fan <qemulist@gmail.com>]

On Thu, Aug 9, 2012 at 4:38 PM, Avi Kivity <avi@redhat.com> wrote:
> On 08/09/2012 10:27 AM, liu ping fan wrote:
>> On Wed, Aug 8, 2012 at 5:20 PM, Avi Kivity <avi@redhat.com> wrote:
>>> On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
>>>> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>>
>>>> Using refcnt for mr, so we can separate mr's life cycle management
>>>> from refered object.
>>>>   When mr->ref 0->1, inc the refered object.
>>>>   When mr->ref 1->0, dec the refered object.
>>>>
>>>> The refered object can be DeviceStae, another mr, or other opaque.
>>>
>>> Please explain the motivation more fully.
>>>
>> Actually, the aim is to mange the reference of an object, used by mem view.
>> DeviceState can be referred by different system, when it comes to the
>> view of subsystem, we hold dev's ref. And any indirect reference will
>> just mr->ref++, not dev's.
>> This can help us avoid the down-walk through the referred chain, like
>> alias----> mr ---> DeviceState.
>
> That is a lot of complexity, for no gain.  Manipulating memory regions
> is a slow path, and can be done under the bit qemu lock without any
> complications.
>
OK. I will discard this design.
>>
>> In the previous discussion, you have suggest add dev->ref++ in
>> core_region_add.  But I think, if we can move it to higher layer --
>> memory_region_{add,del}_subregion, so we can avoid to duplicate do
>> this in other xx_region_add.
>
> Why would other memory listeners be impacted?  They all operate under
> the big qemu lock.  If they start using devices outside the lock, then
> they need to take a reference.
>
Yes, if unplug path in the protection of big lock.
And just one extra question, for ram-unplug scene, how do we protect from:
  updater:  ram-unplug -->qemu free() --> brk() invalidate this vaddr interval
  reader:  vhost-thread copy data from the interval
I guess something like lock/ref used by them, but can not find such
mechanism in vhost_set_memory() to protect the scene against
vhost_worker()

Thanks and regards,
pingfan

>> As a payment for this, we need to handle alias which can be avoid at
>> core_region_add().  And mr's ref can help to avoid
>>  the down-walk.
>
> The payment is two systems of reference counts.
>
> --
> error compiling committee.c: too many arguments to function