* [Qemu-devel] [PATCH] trace: add glib 2.32+ static GMutex support
@ 2013-12-12 14:52 Stefan Hajnoczi
2013-12-12 15:36 ` Michael Tokarev
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2013-12-12 14:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Dr. David Alan Gilbert, Stefan Hajnoczi
The GStaticMutex API was deprecated in glib 2.32. We cannot switch over
to GMutex unconditionally since we would drop support for older glib
versions. But the deprecated API warnings during build are annoying so
use static GMutex when possible.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
trace/simple.c | 45 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/trace/simple.c b/trace/simple.c
index 1e3f691..941f7ea 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -39,7 +39,11 @@
* Trace records are written out by a dedicated thread. The thread waits for
* records to become available, writes them out, and then waits again.
*/
+#if GLIB_CHECK_VERSION(2, 32, 0)
+static GMutex trace_lock;
+#else
static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+#endif
/* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
#if GLIB_CHECK_VERSION(2, 31, 0)
@@ -86,6 +90,34 @@ typedef struct {
static void read_from_buffer(unsigned int idx, void *dataptr, size_t size);
static unsigned int write_to_buffer(unsigned int idx, void *dataptr, size_t size);
+/* Hide changes in glib mutex APIs */
+static void lock_trace_lock(void)
+{
+#if GLIB_CHECK_VERSION(2, 32, 0)
+ g_mutex_lock(&trace_lock);
+#else
+ g_static_mutex_lock(&trace_lock);
+#endif
+}
+
+static void unlock_trace_lock(void)
+{
+#if GLIB_CHECK_VERSION(2, 32, 0)
+ g_mutex_unlock(&trace_lock);
+#else
+ g_static_mutex_unlock(&trace_lock);
+#endif
+}
+
+static GMutex *get_trace_lock_mutex(void)
+{
+#if GLIB_CHECK_VERSION(2, 32, 0)
+ return &trace_lock;
+#else
+ return g_static_mutex_get_mutex(&trace_lock);
+#endif
+}
+
static void clear_buffer_range(unsigned int idx, size_t len)
{
uint32_t num = 0;
@@ -139,27 +171,26 @@ static bool get_trace_record(unsigned int idx, TraceRecord **recordptr)
*/
static void flush_trace_file(bool wait)
{
- g_static_mutex_lock(&trace_lock);
+ lock_trace_lock();
trace_available = true;
g_cond_signal(trace_available_cond);
if (wait) {
- g_cond_wait(trace_empty_cond, g_static_mutex_get_mutex(&trace_lock));
+ g_cond_wait(trace_empty_cond, get_trace_lock_mutex());
}
- g_static_mutex_unlock(&trace_lock);
+ unlock_trace_lock();
}
static void wait_for_trace_records_available(void)
{
- g_static_mutex_lock(&trace_lock);
+ lock_trace_lock();
while (!(trace_available && trace_writeout_enabled)) {
g_cond_signal(trace_empty_cond);
- g_cond_wait(trace_available_cond,
- g_static_mutex_get_mutex(&trace_lock));
+ g_cond_wait(trace_available_cond, get_trace_lock_mutex());
}
trace_available = false;
- g_static_mutex_unlock(&trace_lock);
+ unlock_trace_lock();
}
static gpointer writeout_thread(gpointer opaque)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] trace: add glib 2.32+ static GMutex support
2013-12-12 14:52 [Qemu-devel] [PATCH] trace: add glib 2.32+ static GMutex support Stefan Hajnoczi
@ 2013-12-12 15:36 ` Michael Tokarev
2013-12-13 9:48 ` Stefan Hajnoczi
0 siblings, 1 reply; 3+ messages in thread
From: Michael Tokarev @ 2013-12-12 15:36 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Dr. David Alan Gilbert
12.12.2013 18:52, Stefan Hajnoczi wrote:
> The GStaticMutex API was deprecated in glib 2.32. We cannot switch over
> to GMutex unconditionally since we would drop support for older glib
> versions. But the deprecated API warnings during build are annoying so
> use static GMutex when possible.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> trace/simple.c | 45 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/trace/simple.c b/trace/simple.c
> index 1e3f691..941f7ea 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -39,7 +39,11 @@
> * Trace records are written out by a dedicated thread. The thread waits for
> * records to become available, writes them out, and then waits again.
> */
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> +static GMutex trace_lock;
> +#else
> static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
> +#endif
>
> /* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
> #if GLIB_CHECK_VERSION(2, 31, 0)
> @@ -86,6 +90,34 @@ typedef struct {
> static void read_from_buffer(unsigned int idx, void *dataptr, size_t size);
> static unsigned int write_to_buffer(unsigned int idx, void *dataptr, size_t size);
>
> +/* Hide changes in glib mutex APIs */
> +static void lock_trace_lock(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + g_mutex_lock(&trace_lock);
> +#else
> + g_static_mutex_lock(&trace_lock);
> +#endif
> +}
> +
> +static void unlock_trace_lock(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + g_mutex_unlock(&trace_lock);
> +#else
> + g_static_mutex_unlock(&trace_lock);
> +#endif
> +}
> +
> +static GMutex *get_trace_lock_mutex(void)
> +{
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> + return &trace_lock;
> +#else
> + return g_static_mutex_get_mutex(&trace_lock);
> +#endif
> +}
I'd group mutex definition above with all the functions accessing it,
and also make the functions inline.
Well, to my taste, this is a good example where #define is better than
an inline function. Compare the above with:
diff --git a/trace/simple.c b/trace/simple.c
index 1e3f691..2e55ac1 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -39,7 +39,17 @@
* Trace records are written out by a dedicated thread. The thread waits for
* records to become available, writes them out, and then waits again.
*/
+#if GLIB_CHECK_VERSION(2, 32, 0)
+static GMutex trace_lock;
+#define lock_trace_lock() g_mutex_lock(&trace_lock)
+#define unlock_trace_lock() g_mutex_unlock(&trace_lock)
+#define get_trace_lock_mutex() (&trace_lock)
+#else
static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
+#define lock_trace_lock() g_static_mutex_lock(&trace_lock)
+#define unlock_trace_lock() g_static_mutex_unlock(&trace_lock)
+#define get_trace_lock_mutex() g_static_mutex_get_mutex(&trace_lock)
+#endif
/* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
#if GLIB_CHECK_VERSION(2, 31, 0)
(#defines here and elsewhere has added bonus - when debugging, debugger
does not step into the inline functions, -- such stepping is quite annoying).
But somehow many developers prefer inline functions (sometimes it is better
indeed, especially in a commonly used header files, and when the functions
has complex or many parameters; in this case we have much simpler situation.
For fun, this #ifdeffery is 5 times larger than the actual users of the
functions being defined :)
Thanks,
/mjt
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] trace: add glib 2.32+ static GMutex support
2013-12-12 15:36 ` Michael Tokarev
@ 2013-12-13 9:48 ` Stefan Hajnoczi
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2013-12-13 9:48 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel, Dr. David Alan Gilbert
On Thu, Dec 12, 2013 at 07:36:18PM +0400, Michael Tokarev wrote:
> 12.12.2013 18:52, Stefan Hajnoczi wrote:
> > The GStaticMutex API was deprecated in glib 2.32. We cannot switch over
> > to GMutex unconditionally since we would drop support for older glib
> > versions. But the deprecated API warnings during build are annoying so
> > use static GMutex when possible.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > trace/simple.c | 45 ++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/trace/simple.c b/trace/simple.c
> > index 1e3f691..941f7ea 100644
> > --- a/trace/simple.c
> > +++ b/trace/simple.c
> > @@ -39,7 +39,11 @@
> > * Trace records are written out by a dedicated thread. The thread waits for
> > * records to become available, writes them out, and then waits again.
> > */
> > +#if GLIB_CHECK_VERSION(2, 32, 0)
> > +static GMutex trace_lock;
> > +#else
> > static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
> > +#endif
> >
> > /* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
> > #if GLIB_CHECK_VERSION(2, 31, 0)
> > @@ -86,6 +90,34 @@ typedef struct {
> > static void read_from_buffer(unsigned int idx, void *dataptr, size_t size);
> > static unsigned int write_to_buffer(unsigned int idx, void *dataptr, size_t size);
> >
> > +/* Hide changes in glib mutex APIs */
> > +static void lock_trace_lock(void)
> > +{
> > +#if GLIB_CHECK_VERSION(2, 32, 0)
> > + g_mutex_lock(&trace_lock);
> > +#else
> > + g_static_mutex_lock(&trace_lock);
> > +#endif
> > +}
> > +
> > +static void unlock_trace_lock(void)
> > +{
> > +#if GLIB_CHECK_VERSION(2, 32, 0)
> > + g_mutex_unlock(&trace_lock);
> > +#else
> > + g_static_mutex_unlock(&trace_lock);
> > +#endif
> > +}
> > +
> > +static GMutex *get_trace_lock_mutex(void)
> > +{
> > +#if GLIB_CHECK_VERSION(2, 32, 0)
> > + return &trace_lock;
> > +#else
> > + return g_static_mutex_get_mutex(&trace_lock);
> > +#endif
> > +}
>
>
> I'd group mutex definition above with all the functions accessing it,
> and also make the functions inline.
>
> Well, to my taste, this is a good example where #define is better than
> an inline function. Compare the above with:
>
> diff --git a/trace/simple.c b/trace/simple.c
> index 1e3f691..2e55ac1 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -39,7 +39,17 @@
> * Trace records are written out by a dedicated thread. The thread waits for
> * records to become available, writes them out, and then waits again.
> */
> +#if GLIB_CHECK_VERSION(2, 32, 0)
> +static GMutex trace_lock;
> +#define lock_trace_lock() g_mutex_lock(&trace_lock)
> +#define unlock_trace_lock() g_mutex_unlock(&trace_lock)
> +#define get_trace_lock_mutex() (&trace_lock)
> +#else
> static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
> +#define lock_trace_lock() g_static_mutex_lock(&trace_lock)
> +#define unlock_trace_lock() g_static_mutex_unlock(&trace_lock)
> +#define get_trace_lock_mutex() g_static_mutex_get_mutex(&trace_lock)
> +#endif
>
> /* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
> #if GLIB_CHECK_VERSION(2, 31, 0)
>
> (#defines here and elsewhere has added bonus - when debugging, debugger
> does not step into the inline functions, -- such stepping is quite annoying).
>
> But somehow many developers prefer inline functions (sometimes it is better
> indeed, especially in a commonly used header files, and when the functions
> has complex or many parameters; in this case we have much simpler situation.
>
> For fun, this #ifdeffery is 5 times larger than the actual users of the
> functions being defined :)
Yes, I think you are right. In general I avoid using macros but here it
does make things nicer.
Stefan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-13 9:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-12 14:52 [Qemu-devel] [PATCH] trace: add glib 2.32+ static GMutex support Stefan Hajnoczi
2013-12-12 15:36 ` Michael Tokarev
2013-12-13 9:48 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).