* [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.