Re: [PATCH v2] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it.

[朱光宏 <zhuguanghong@uniontech.com>]

[-- Attachment #1: Type: text/plain, Size: 3385 bytes --]

 Hi
Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com&gt;
---
&nbsp;qom/object.c | 4 ++++
&nbsp;1 file changed, 4 insertions(+)


diff --git a/qom/object.c b/qom/object.c
index 6a01d56546..dc0a363ed0 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -73,15 +73,19 @@ struct TypeImpl
&nbsp;};


&nbsp;static Type type_interface;
+G_LOCK_DEFINE (GHashTable_mutex);


&nbsp;static GHashTable *type_table_get(void)
&nbsp;{
&nbsp; &nbsp; &nbsp;static GHashTable *type_table;
+&nbsp; &nbsp; G_LOCK ( GHashTable_mutex);&nbsp; &nbsp; &nbsp; &nbsp;&nbsp;


&nbsp; &nbsp; &nbsp;if (type_table == NULL) {
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;type_table = g_hash_table_new(g_str_hash, g_str_equal);
&nbsp; &nbsp; &nbsp;}


+&nbsp; &nbsp; G_UNLOCK ( GHashTable_mutex);&nbsp; &nbsp; &nbsp;&nbsp;
+
&nbsp; &nbsp; &nbsp;return type_table;
&nbsp;}



How about using G_LOCK ? thread safety




&nbsp;
&nbsp;
------------------&nbsp;Original&nbsp;------------------
From: &nbsp;"Daniel P. Berrangé"<berrange@redhat.com&gt;;
Date: &nbsp;Wed, Jul 21, 2021 10:00 PM
To: &nbsp;"Marc-André Lureau"<marcandre.lureau@gmail.com&gt;; 
Cc: &nbsp;"zhuguanghong"<zhuguanghong@uniontech.com&gt;; "Paolo Bonzini"<pbonzini@redhat.com&gt;; "Eduardo Habkost"<ehabkost@redhat.com&gt;; "QEMU"<qemu-devel@nongnu.org&gt;; 
Subject: &nbsp;Re: [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it.

&nbsp;

On Wed, Jul 21, 2021 at 05:55:44PM +0400, Marc-André Lureau wrote:
&gt; Hi
&gt; 
&gt; On Wed, Jul 21, 2021 at 5:22 PM zhuguanghong <zhuguanghong@uniontech.com&gt;
&gt; wrote:
&gt; 
&gt; &gt; Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com&gt;
&gt; &gt; ---
&gt; &gt;&nbsp; qom/object.c | 4 +---
&gt; &gt;&nbsp; 1 file changed, 1 insertion(+), 3 deletions(-)
&gt; &gt;
&gt; &gt; diff --git a/qom/object.c b/qom/object.c
&gt; &gt; index 6a01d56546..c8f5481afe 100644
&gt; &gt; --- a/qom/object.c
&gt; &gt; +++ b/qom/object.c
&gt; &gt; @@ -78,9 +78,7 @@ static GHashTable *type_table_get(void)
&gt; &gt;&nbsp; {
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; static GHashTable *type_table;
&gt; &gt;
&gt; &gt; -&nbsp;&nbsp;&nbsp; if (type_table == NULL) {
&gt; &gt; -&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; type_table = g_hash_table_new(g_str_hash, g_str_equal);
&gt; &gt; -&nbsp;&nbsp;&nbsp; }
&gt; &gt; +&nbsp;&nbsp;&nbsp; type_table = g_hash_table_new(g_str_hash, g_str_equal);
&gt; &gt;
&gt; &gt;
&gt; nack.It's not redundant, it does a one-time initialization.
&gt; 
&gt; We may want to replace it with a more explicit and thread-safe version
&gt; though:
&gt; https://developer.gnome.org/glib/stable/glib-Threads.html#g-once-init-enter

I would have thought QOM usage is so widespread in QEMU that we'll
trigger initialization of this hash tble very early in startup
while we're still single threaded.

That said there's not really any harm in using g_once

Regards,
Daniel
-- 
|: https://berrange.com&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; -o-&nbsp;&nbsp;&nbsp; https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; -o-&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; https://fstop138.berrange.com :|
|: https://entangle-photo.org&nbsp;&nbsp;&nbsp; -o-&nbsp;&nbsp;&nbsp; https://www.instagram.com/dberrange :|

[-- Attachment #2: Type: text/html, Size: 4476 bytes --]