* [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. @ 2021-07-21 8:44 zhuguanghong 2021-07-21 13:55 ` Marc-André Lureau 0 siblings, 1 reply; 5+ messages in thread From: zhuguanghong @ 2021-07-21 8:44 UTC (permalink / raw) To: pbonzini, berrange, ehabkost; +Cc: zhuguanghong, qemu-devel Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> --- qom/object.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qom/object.c b/qom/object.c index 6a01d56546..c8f5481afe 100644 --- a/qom/object.c +++ b/qom/object.c @@ -78,9 +78,7 @@ static GHashTable *type_table_get(void) { static GHashTable *type_table; - if (type_table == NULL) { - type_table = g_hash_table_new(g_str_hash, g_str_equal); - } + type_table = g_hash_table_new(g_str_hash, g_str_equal); return type_table; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. 2021-07-21 8:44 [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it zhuguanghong @ 2021-07-21 13:55 ` Marc-André Lureau 2021-07-21 13:59 ` Daniel P. Berrangé 0 siblings, 1 reply; 5+ messages in thread From: Marc-André Lureau @ 2021-07-21 13:55 UTC (permalink / raw) To: zhuguanghong; +Cc: Paolo Bonzini, Daniel P. Berrange, Eduardo Habkost, QEMU [-- Attachment #1: Type: text/plain, Size: 968 bytes --] Hi On Wed, Jul 21, 2021 at 5:22 PM zhuguanghong <zhuguanghong@uniontech.com> wrote: > Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> > --- > qom/object.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 6a01d56546..c8f5481afe 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -78,9 +78,7 @@ static GHashTable *type_table_get(void) > { > static GHashTable *type_table; > > - if (type_table == NULL) { > - type_table = g_hash_table_new(g_str_hash, g_str_equal); > - } > + type_table = g_hash_table_new(g_str_hash, g_str_equal); > > nack.It's not redundant, it does a one-time initialization. We may want to replace it with a more explicit and thread-safe version though: https://developer.gnome.org/glib/stable/glib-Threads.html#g-once-init-enter return type_table; > } > -- > 2.20.1 > > > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 1773 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. 2021-07-21 13:55 ` Marc-André Lureau @ 2021-07-21 13:59 ` Daniel P. Berrangé 2021-07-22 9:15 ` [PATCH v2] " 朱光宏 0 siblings, 1 reply; 5+ messages in thread From: Daniel P. Berrangé @ 2021-07-21 13:59 UTC (permalink / raw) To: Marc-André Lureau; +Cc: QEMU, zhuguanghong, Eduardo Habkost, Paolo Bonzini On Wed, Jul 21, 2021 at 05:55:44PM +0400, Marc-André Lureau wrote: > Hi > > On Wed, Jul 21, 2021 at 5:22 PM zhuguanghong <zhuguanghong@uniontech.com> > wrote: > > > Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> > > --- > > qom/object.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/qom/object.c b/qom/object.c > > index 6a01d56546..c8f5481afe 100644 > > --- a/qom/object.c > > +++ b/qom/object.c > > @@ -78,9 +78,7 @@ static GHashTable *type_table_get(void) > > { > > static GHashTable *type_table; > > > > - if (type_table == NULL) { > > - type_table = g_hash_table_new(g_str_hash, g_str_equal); > > - } > > + type_table = g_hash_table_new(g_str_hash, g_str_equal); > > > > > nack.It's not redundant, it does a one-time initialization. > > We may want to replace it with a more explicit and thread-safe version > though: > 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 -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. 2021-07-21 13:59 ` Daniel P. Berrangé @ 2021-07-22 9:15 ` 朱光宏 2021-07-22 9:19 ` Daniel P. Berrangé 0 siblings, 1 reply; 5+ messages in thread From: 朱光宏 @ 2021-07-22 9:15 UTC (permalink / raw) To: Daniel P. Berrangé, Marc-André Lureau Cc: Paolo Bonzini, Eduardo Habkost, QEMU [-- Attachment #1: Type: text/plain, Size: 3385 bytes --] Hi Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> --- qom/object.c | 4 ++++ 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 }; static Type type_interface; +G_LOCK_DEFINE (GHashTable_mutex); static GHashTable *type_table_get(void) { static GHashTable *type_table; + G_LOCK ( GHashTable_mutex); if (type_table == NULL) { type_table = g_hash_table_new(g_str_hash, g_str_equal); } + G_UNLOCK ( GHashTable_mutex); + return type_table; } How about using G_LOCK ? thread safety ------------------ Original ------------------ From: "Daniel P. Berrangé"<berrange@redhat.com>; Date: Wed, Jul 21, 2021 10:00 PM To: "Marc-André Lureau"<marcandre.lureau@gmail.com>; Cc: "zhuguanghong"<zhuguanghong@uniontech.com>; "Paolo Bonzini"<pbonzini@redhat.com>; "Eduardo Habkost"<ehabkost@redhat.com>; "QEMU"<qemu-devel@nongnu.org>; Subject: Re: [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. On Wed, Jul 21, 2021 at 05:55:44PM +0400, Marc-André Lureau wrote: > Hi > > On Wed, Jul 21, 2021 at 5:22 PM zhuguanghong <zhuguanghong@uniontech.com> > wrote: > > > Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> > > --- > > qom/object.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/qom/object.c b/qom/object.c > > index 6a01d56546..c8f5481afe 100644 > > --- a/qom/object.c > > +++ b/qom/object.c > > @@ -78,9 +78,7 @@ static GHashTable *type_table_get(void) > > { > > static GHashTable *type_table; > > > > - if (type_table == NULL) { > > - type_table = g_hash_table_new(g_str_hash, g_str_equal); > > - } > > + type_table = g_hash_table_new(g_str_hash, g_str_equal); > > > > > nack.It's not redundant, it does a one-time initialization. > > We may want to replace it with a more explicit and thread-safe version > though: > 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 -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| [-- Attachment #2: Type: text/html, Size: 4476 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it. 2021-07-22 9:15 ` [PATCH v2] " 朱光宏 @ 2021-07-22 9:19 ` Daniel P. Berrangé 0 siblings, 0 replies; 5+ messages in thread From: Daniel P. Berrangé @ 2021-07-22 9:19 UTC (permalink / raw) To: 朱光宏 Cc: Paolo Bonzini, Marc-André Lureau, Eduardo Habkost, QEMU On Thu, Jul 22, 2021 at 05:15:46PM +0800, 朱光宏 wrote: > Hi > Signed-off-by: zhuguanghong <zhuguanghong@uniontech.com> > --- > qom/object.c | 4 ++++ > 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 > }; > > > static Type type_interface; > +G_LOCK_DEFINE (GHashTable_mutex); > > > static GHashTable *type_table_get(void) > { > static GHashTable *type_table; > + G_LOCK ( GHashTable_mutex); > > > if (type_table == NULL) { > type_table = g_hash_table_new(g_str_hash, g_str_equal); > } > > > + G_UNLOCK ( GHashTable_mutex); > + > return type_table; > } Please could you check your mail client settings so that it correctly sends *plain* text messages. This is a multipart text/plain+text/html message, but the text/plain part is full of HTML entities. This makes your messages almost impossible to read and reply to. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-22 9:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-21 8:44 [PATCH] qom/object.c 'if (type_table == NULL)' statement is redundant , delete it zhuguanghong 2021-07-21 13:55 ` Marc-André Lureau 2021-07-21 13:59 ` Daniel P. Berrangé 2021-07-22 9:15 ` [PATCH v2] " 朱光宏 2021-07-22 9:19 ` Daniel P. Berrangé
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.