Re: [Qemu-devel] [patch v5 5/8] memory: introduce local lock for address space

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

On Mon, Oct 29, 2012 at 5:32 PM, Avi Kivity <avi@redhat.com> wrote:
> On 10/29/2012 01:48 AM, Liu Ping Fan wrote:
>> For those address spaces which want to be able out of big lock, they
>> will be protected by their own local.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>>  memory.c |   11 ++++++++++-
>>  memory.h |    5 ++++-
>>  2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index 2f68d67..ff34aed 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1532,9 +1532,15 @@ void memory_listener_unregister(MemoryListener *listener)
>>      QTAILQ_REMOVE(&memory_listeners, listener, link);
>>  }
>>
>> -void address_space_init(AddressSpace *as, MemoryRegion *root)
>> +void address_space_init(AddressSpace *as, MemoryRegion *root, bool lock)
>
>
> Why not always use the lock?  Even if the big lock is taken, it doesn't
> hurt.  And eventually all address spaces will be fine-grained.
>
I had thought only mmio is out of big lock's protection. While others
address space will take extra expense. So leave them until they are
ready to be out of big lock.

>>  {
>>      memory_region_transaction_begin();
>> +    if (lock) {
>> +        as->lock = g_new(QemuMutex, 1);
>> +        qemu_mutex_init(as->lock);
>> +    } else {
>> +        as->lock = NULL;
>> +    }
>>      as->root = root;
>>      as->current_map = g_new(FlatView, 1);
>>      flatview_init(as->current_map);
>> @@ -1553,6 +1559,9 @@ void address_space_destroy(AddressSpace *as)
>>      QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
>>      address_space_destroy_dispatch(as);
>>      flatview_destroy(as->current_map);
>> +    if (as->lock) {
>> +        g_free(as->lock);
>> +    }
>>      g_free(as->current_map);
>>  }
>>
>> diff --git a/memory.h b/memory.h
>> index 79393f1..12d1c56 100644
>> --- a/memory.h
>> +++ b/memory.h
>> @@ -22,6 +22,7 @@
>>  #include "cpu-common.h"
>>  #include "targphys.h"
>>  #include "qemu-queue.h"
>> +#include "qemu-thread.h"
>>  #include "iorange.h"
>>  #include "ioport.h"
>>  #include "int128.h"
>> @@ -164,6 +165,7 @@ typedef struct AddressSpace AddressSpace;
>>   */
>>  struct AddressSpace {
>>      /* All fields are private. */
>> +    QemuMutex *lock;
>>      const char *name;
>>      MemoryRegion *root;
>>      struct FlatView *current_map;
>> @@ -801,8 +803,9 @@ void mtree_info(fprintf_function mon_printf, void *f);
>>   *
>>   * @as: an uninitialized #AddressSpace
>>   * @root: a #MemoryRegion that routes addesses for the address space
>> + * @lock: if true, the physmap protected by local lock, otherwise big lock
>>   */
>> -void address_space_init(AddressSpace *as, MemoryRegion *root);
>> +void address_space_init(AddressSpace *as, MemoryRegion *root, bool lock);
>>
>>
>>  /**
>>
>
>
> --
> error compiling committee.c: too many arguments to function
>