Re: [waffle] [PATCH 2/3] wcore: rework non-compatible mutex initialization
On 20 March 2015 at 11:46, Jose Fonseca wrote:
> On 19/03/15 22:26, Emil Velikov wrote:
>>
>> C11 does not specify a static initializer, based on the idea that the
>> a mutex will be platform and/or implementation dependent. As such the
>> alternative solution is to initialize the mutex with mtx_init().
>> This will allow us to remove the transition hack, and in due time move
>> to the system's C11 threads implementation.
>>
>> Signed-off-by: Emil Velikov
>> ---
>> src/waffle/core/wcore_display.c | 6 +++---
>> src/waffle/core/wcore_display.h | 3 ++-
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/waffle/core/wcore_display.c
>> b/src/waffle/core/wcore_display.c
>> index 2feeeba..49aeae6 100644
>> --- a/src/waffle/core/wcore_display.c
>> +++ b/src/waffle/core/wcore_display.c
>> @@ -34,14 +34,14 @@ wcore_display_init(struct wcore_display *self,
>> struct wcore_platform *platform)
>> {
>> static size_t id_counter = 0;
>> -static mtx_t mutex = _MTX_INITIALIZER_NP;
>
>
> IIUC, this mutex protecting the static id_counter above, not the
> wcore_display object, or anything, so moving mtx to display seems incorrect.
>
> The ideal solution here would be to use C11 atomics, but that's a bunch of
> extra portibility code to maintain... The portable way of fixing this with
> C11 threads is using once_flag.
>
Agreed, using C11 atomics was something that came to mind. Although
I'd rather avoid adding another more compat layer.
I see that keeping the mutex within struct wcore_display was a bad
idea. And it's the one lead me to drop the call_once in the first
place. Upon closer look it seems that
wcore_error_unittest.c:test_wcore_error_thread_local() will need a
similar treatment.
As always you're a star Jose. Thanks.
Emil
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 2/3] wcore: rework non-compatible mutex initialization
On 19/03/15 22:26, Emil Velikov wrote:
C11 does not specify a static initializer, based on the idea that the
a mutex will be platform and/or implementation dependent. As such the
alternative solution is to initialize the mutex with mtx_init().
This will allow us to remove the transition hack, and in due time move
to the system's C11 threads implementation.
Signed-off-by: Emil Velikov
---
src/waffle/core/wcore_display.c | 6 +++---
src/waffle/core/wcore_display.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/waffle/core/wcore_display.c b/src/waffle/core/wcore_display.c
index 2feeeba..49aeae6 100644
--- a/src/waffle/core/wcore_display.c
+++ b/src/waffle/core/wcore_display.c
@@ -34,14 +34,14 @@ wcore_display_init(struct wcore_display *self,
struct wcore_platform *platform)
{
static size_t id_counter = 0;
-static mtx_t mutex = _MTX_INITIALIZER_NP;
IIUC, this mutex protecting the static id_counter above, not the
wcore_display object, or anything, so moving mtx to display seems incorrect.
The ideal solution here would be to use C11 atomics, but that's a bunch
of extra portibility code to maintain... The portable way of fixing
this with C11 threads is using once_flag.
Jose
assert(self);
assert(platform);
-mtx_lock(&mutex);
+mtx_init(&self->mutex, mtx_plain);
+mtx_lock(&self->mutex);
self->api.display_id = ++id_counter;
-mtx_unlock(&mutex);
+mtx_unlock(&self->mutex);
self->platform = platform;
diff --git a/src/waffle/core/wcore_display.h b/src/waffle/core/wcore_display.h
index de6ca5e..c4348c4 100644
--- a/src/waffle/core/wcore_display.h
+++ b/src/waffle/core/wcore_display.h
@@ -39,6 +39,7 @@ union waffle_native_display;
struct wcore_display {
struct api_object api;
struct wcore_platform *platform;
+mtx_t mutex;
};
static inline struct waffle_display*
@@ -59,7 +60,7 @@ wcore_display_init(struct wcore_display *self,
static inline bool
wcore_display_teardown(struct wcore_display *self)
{
-(void) self;
assert(self);
+mtx_destroy(&self->mutex);
return true;
}
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle
[waffle] [PATCH 2/3] wcore: rework non-compatible mutex initialization
C11 does not specify a static initializer, based on the idea that the
a mutex will be platform and/or implementation dependent. As such the
alternative solution is to initialize the mutex with mtx_init().
This will allow us to remove the transition hack, and in due time move
to the system's C11 threads implementation.
Signed-off-by: Emil Velikov
---
src/waffle/core/wcore_display.c | 6 +++---
src/waffle/core/wcore_display.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/waffle/core/wcore_display.c b/src/waffle/core/wcore_display.c
index 2feeeba..49aeae6 100644
--- a/src/waffle/core/wcore_display.c
+++ b/src/waffle/core/wcore_display.c
@@ -34,14 +34,14 @@ wcore_display_init(struct wcore_display *self,
struct wcore_platform *platform)
{
static size_t id_counter = 0;
-static mtx_t mutex = _MTX_INITIALIZER_NP;
assert(self);
assert(platform);
-mtx_lock(&mutex);
+mtx_init(&self->mutex, mtx_plain);
+mtx_lock(&self->mutex);
self->api.display_id = ++id_counter;
-mtx_unlock(&mutex);
+mtx_unlock(&self->mutex);
self->platform = platform;
diff --git a/src/waffle/core/wcore_display.h b/src/waffle/core/wcore_display.h
index de6ca5e..c4348c4 100644
--- a/src/waffle/core/wcore_display.h
+++ b/src/waffle/core/wcore_display.h
@@ -39,6 +39,7 @@ union waffle_native_display;
struct wcore_display {
struct api_object api;
struct wcore_platform *platform;
+mtx_t mutex;
};
static inline struct waffle_display*
@@ -59,7 +60,7 @@ wcore_display_init(struct wcore_display *self,
static inline bool
wcore_display_teardown(struct wcore_display *self)
{
-(void) self;
assert(self);
+mtx_destroy(&self->mutex);
return true;
}
--
2.3.1
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle
