Re: [Qemu-devel] [PATCH] RAMList: replace QemuMutex with CompatGMutex

[Paolo Bonzini <pbonzini@redhat.com>]

On 14/02/2018 12:43, Zihan Yang wrote:
> 
>> I'm not sure what is the advantage of this change.  QEMU is more or less
>> uniformly using QemuMutex and QemuCond.
> 
> I see. QEMU uses some glib functions, like g_new, g_free, g_hash_table ,
> g_assert and g_poll, so I thought there was a trend for making more use
> of glib.
> I get the point now.

There are some plans about using priority inheritance mutexes for
QemuMutex.  GLib does not support anymore pluggable thread functions, so
we are stuck with our own implementation.

Paolo

> 2018-02-14 18:13 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>>:
> 
>     On 14/02/2018 09:49, Zihan Yang wrote:
>     > change the mutex member in RAMList structure from QemuMutex to CompatGMutex.
>     > qemu_mutex_init() is just deleted instead of being replaced with g_mutex_init()
>     > because there is no need to do initialize a mutex that is statically allocated.
>     >
>     > Signed-off-by: Zihan Yang <whois.zihan.yang@gmail.com <mailto:whois.zihan.yang@gmail.com>>
> 
>     I'm not sure what is the advantage of this change.  QEMU is more or less
>     uniformly using QemuMutex and QemuCond.
> 
>     Paolo
> 
>     > ---
>     >  exec.c                 | 5 ++---
>     >  include/exec/ramlist.h | 2 +-
>     >  2 files changed, 3 insertions(+), 4 deletions(-)
>     >
>     > diff --git a/exec.c b/exec.c
>     > index e8d7b33..90cdd05 100644
>     > --- a/exec.c
>     > +++ b/exec.c
>     > @@ -1423,12 +1423,12 @@ void qemu_flush_coalesced_mmio_buffer(void)
>     >
>     >  void qemu_mutex_lock_ramlist(void)
>     >  {
>     > -    qemu_mutex_lock(&ram_list.mutex);
>     > +    g_mutex_lock(&ram_list.mutex);
>     >  }
>     >
>     >  void qemu_mutex_unlock_ramlist(void)
>     >  {
>     > -    qemu_mutex_unlock(&ram_list.mutex);
>     > +    g_mutex_unlock(&ram_list.mutex);
>     >  }
>     >
>     >  void ram_block_dump(Monitor *mon)
>     > @@ -3331,7 +3331,6 @@ void cpu_register_map_client(QEMUBH *bh)
>     >
>     >  void cpu_exec_init_all(void)
>     >  {
>     > -    qemu_mutex_init(&ram_list.mutex);
>     >      /* The data structures we set up here depend on knowing the
>     page size,
>     >       * so no more changes can be made after this point.
>     >       * In an ideal world, nothing we did before we had finished the
>     > diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
>     > index 2e2ac6c..24be5e4 100644
>     > --- a/include/exec/ramlist.h
>     > +++ b/include/exec/ramlist.h
>     > @@ -45,7 +45,7 @@ typedef struct {
>     >  } DirtyMemoryBlocks;
>     >
>     >  typedef struct RAMList {
>     > -    QemuMutex mutex;
>     > +    CompatGMutex mutex;
>     >      RAMBlock *mru_block;
>     >      /* RCU-enabled, writes protected by the ramlist lock. */
>     >      QLIST_HEAD(, RAMBlock) blocks;
>     >
> 
>